From 8ccc377cd6ad066a317ef9fcccbdfa8fcaca9c35 Mon Sep 17 00:00:00 2001 From: bladehell-ai Date: Fri, 21 Aug 2026 19:59:28 +0500 Subject: [PATCH] fix(reports): report one worked-time total across every surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same date range produced four different answers. Measured org-wide over one 60-day window: reports tab 23.02h, report builder 31.52h, dashboard 31.51h, Time page 31.68h — of which the Time page displayed only the slice on screen (3905h against a true 2001h across the full dataset). There was no shared definition of "worked time", and the four surfaces disagreed on all three axes at once: * Which rows count — analytics filtered `type='tracked'` for its headline KPI, dropping approved MANUAL time, while its own charts and log tables filtered nothing and counted IDLE. A table never reconciled with its own header. * How long a row is — the dashboard summed `duration_seconds`, a desktop-written column that is not trustworthy: idle rows store 1897h against 55h of real wall-clock, and several tracked rows store NEGATIVE seconds, which SUM() silently subtracts. * Which day it lands on — summary() grouped by `DATE(started_at)`, i.e. the UTC date, inside range bounds resolved in the org timezone. For Asia/Karachi that pushed work done before 05:00 local onto the previous day's bar. And the Time page summed the page it had been handed (20 rows) beside an entry count covering every match, so the card moved as you paged. App\Support\WorkedTime is now the single definition — clamped timestamp-derived duration, org-timezone day bucket, approved-non-idle scope — wired through ReportService, DashboardController, and TimeEntryController, which now returns total_seconds/idle_seconds/unapproved_seconds for the WHOLE filtered set instead of letting the client sum a page. fix(reports): honour the report user filter by permission scope, not role name Picking any user in the Advanced Report Builder — or "All Users" — returned the logged-in user's own figures. `reports.view` is granted to the employee role at scope 'own', so an employee legitimately reaches these endpoints. The web client gated the user picker on merely HOLDING the permission, so it rendered a full team dropdown, sent user_id, and the API then clamped every request back to the caller. The UI advertised a capability the API was never going to honour. The server side was brittle too: it decided visibility with `$user->isEmployee()`, which reads the `users.role` STRING — a fallback used only when a user has no user_roles row, on a role set that is open because orgs define custom roles. ReportController now resolves visibility from the SCOPE on the actor's grant: organization sees anyone, project sees their team (a requested outsider narrows to self rather than widening), own is clamped to self. ReportService accepts a list of user ids so project scope is expressible instead of falling through to the whole org. The reports page gates the picker on hasPermissionWithScope('reports.view','project') — the same test the Time page already uses — so own-scoped users get no dropdown at all. Verified with real dispatched requests: owner sees 15.58h all / 13.71h for a chosen member; an own-scoped employee gets their own 1.87h whatever they ask. Co-Authored-By: Claude Opus 5 --- .../Api/V1/DashboardController.php | 14 +- .../Controllers/Api/V1/ReportController.php | 109 +++++--- .../Api/V1/TimeEntryController.php | 31 ++- backend/app/Services/ReportService.php | 113 +++++--- backend/app/Support/WorkedTime.php | 91 +++++++ .../Timer/WorkedTimeConsistencyTest.php | 247 ++++++++++++++++++ bugs/README.md | 1 + ...eb-time-totals-disagree-across-surfaces.md | 92 +++++++ web/src/app/(dashboard)/reports/page.tsx | 131 ++++++---- web/src/app/(dashboard)/time/page.tsx | 13 +- 10 files changed, 711 insertions(+), 131 deletions(-) create mode 100644 backend/app/Support/WorkedTime.php create mode 100644 backend/tests/Feature/Timer/WorkedTimeConsistencyTest.php create mode 100644 bugs/web-time-totals-disagree-across-surfaces.md diff --git a/backend/app/Http/Controllers/Api/V1/DashboardController.php b/backend/app/Http/Controllers/Api/V1/DashboardController.php index a28e46c7..8c692c0e 100644 --- a/backend/app/Http/Controllers/Api/V1/DashboardController.php +++ b/backend/app/Http/Controllers/Api/V1/DashboardController.php @@ -6,6 +6,7 @@ use App\Models\TimeEntry; use App\Models\User; use App\Support\TimezoneAwareDateRange; +use App\Support\WorkedTime; use Carbon\Carbon; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -118,7 +119,7 @@ public function index(Request $request): JsonResponse ->whereNotNull('ended_at') ->where('type', '!=', 'idle') ->where('approval_status', 'approved') - ->selectRaw('user_id, SUM(duration_seconds) as total_seconds') + ->selectRaw('user_id, SUM(' . WorkedTime::durationExpr() . ') as total_seconds') ->groupBy('user_id') ->get() ->keyBy('user_id'); @@ -239,7 +240,8 @@ private function employeeDashboard(User $user, Request $request): JsonResponse ->whereNotNull('ended_at') ->where('type', '!=', 'idle') ->where('approval_status', 'approved') - ->sum('duration_seconds'); + ->selectRaw('COALESCE(SUM(' . WorkedTime::durationExpr() . '), 0) as total_seconds') + ->value('total_seconds'); $now = Carbon::now(); if ($now >= Carbon::parse($dateFrom) && $now < Carbon::parse($dateTo) && $timer) { @@ -282,7 +284,8 @@ private function employeeDashboard(User $user, Request $request): JsonResponse ->whereNotNull('ended_at') ->where('type', '!=', 'idle') ->where('approval_status', 'approved') - ->sum('duration_seconds'); + ->selectRaw('COALESCE(SUM(' . WorkedTime::durationExpr() . '), 0) as total_seconds') + ->value('total_seconds'); // Include current running timer in weekly total only if it started within this week. // Without the boundary check a timer that started in a previous week would add its @@ -312,8 +315,9 @@ private function employeeDashboard(User $user, Request $request): JsonResponse ->where('started_at', '<', $dayEndUtc) ->whereNotNull('ended_at') ->where('type', '!=', 'idle') - ->where('approval_status', 'approved') - ->sum('duration_seconds'); + ->where('approval_status', 'approved') + ->selectRaw('COALESCE(SUM(' . WorkedTime::durationExpr() . '), 0) as total_seconds') + ->value('total_seconds'); // Add running timer elapsed to today's bar if ($dayStr === $todayLocal && $timer) { diff --git a/backend/app/Http/Controllers/Api/V1/ReportController.php b/backend/app/Http/Controllers/Api/V1/ReportController.php index 0040cf17..4983c2ea 100644 --- a/backend/app/Http/Controllers/Api/V1/ReportController.php +++ b/backend/app/Http/Controllers/Api/V1/ReportController.php @@ -3,6 +3,7 @@ namespace App\Http\Controllers\Api\V1; use App\Http\Controllers\Controller; +use App\Services\PermissionService; use App\Services\ReportService; use App\Support\ReportExportFormatter; use App\Support\TimezoneAwareDateRange; @@ -25,6 +26,65 @@ private function parseDateRange(Request $request): array ); } + + /** + * Which user(s) the actor is allowed to see, honouring the requested filter. + * + * Returns a single id, a list of ids, or null for "everyone in the organization". + * + * The decision comes from the SCOPE on the actor's `reports.view` grant, never + * from a role name. Two reasons the old `$user->isEmployee()` test was wrong: + * + * - `reports.view` is granted to the `employee` role at scope `own`, so an + * employee legitimately reaches these endpoints. The web client gates the + * user picker on merely HOLDING the permission, so it rendered a full team + * dropdown, sent `user_id`, and the controller then silently clamped every + * request back to the caller. Picking any user — or "All Users" — returned + * the logged-in user's own figures. + * - `isEmployee()` reads the `users.role` STRING, which is only a fallback for + * users with no `user_roles` row, and the role set is open (orgs define custom + * roles). Deciding data visibility from it is wrong in both directions. + */ + /** + * May the actor see anyone other than themselves? Team, projects, payroll and + * attendance reports are inherently about other people, so an `own`-scoped + * actor has nothing to see in them. + */ + private function canViewOthers(Request $request): bool + { + $user = $request->user(); + + return ($user->getRawOriginal('role') ?? '') === 'owner' + || app(PermissionService::class)->hasPermission($user, 'reports.view', 'project'); + } + + private function scopedUserId(Request $request): string|array|null + { + $user = $request->user(); + $requested = $request->input('user_id') ?: null; + $scope = app(PermissionService::class)->getScope($user, 'reports.view'); + + // Owners bypass the permission map entirely, as they do everywhere else. + if ($scope === 'organization' || ($user->getRawOriginal('role') ?? '') === 'owner') { + return $requested; + } + + if ($scope === 'project') { + $visible = app(PermissionService::class)->getProjectUserIds($user); + + // A requested user outside the actor's team narrows to nothing rather + // than widening the scope — same contract as ProjectTimeReportService. + if ($requested !== null) { + return in_array($requested, $visible, true) ? $requested : $user->id; + } + + return $visible; + } + + // 'own', or no grant at all. + return $user->id; + } + // REPT-01: Summary public function summary(Request $request): JsonResponse { @@ -35,11 +95,7 @@ public function summary(Request $request): JsonResponse ]); $user = $request->user(); - $userId = $request->user_id; - - if ($user->isEmployee()) { - $userId = $user->id; - } + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); @@ -56,7 +112,7 @@ public function summary(Request $request): JsonResponse // REPT-02: Team public function team(Request $request): JsonResponse { - if ($request->user()->isEmployee()) { + if (! $this->canViewOthers($request)) { return response()->json(['message' => 'Unauthorized.'], 403); } @@ -104,10 +160,7 @@ public function apps(Request $request): JsonResponse 'user_id' => 'nullable|uuid', ]); - $userId = $request->user_id; - if ($request->user()->isEmployee()) { - $userId = $request->user()->id; - } + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); @@ -130,10 +183,10 @@ public function timeline(Request $request): JsonResponse 'date' => 'required|date', ]); - $userId = $request->user_id; - if ($request->user()->isEmployee()) { - $userId = $request->user()->id; - } + // `user_id` is required here, so the resolver always yields a single id + // (it only returns a list when no specific user was asked for). + $scoped = $this->scopedUserId($request); + $userId = is_array($scoped) ? $request->user()->id : ($scoped ?? $request->user()->id); $data = $this->reportService->timeline( $request->user()->organization_id, @@ -160,7 +213,7 @@ public function export(Request $request): Response $orgId = $user->organization_id; // Employees can only ever export their own data. - $userId = $user->isEmployee() ? $user->id : $request->user_id; + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); @@ -221,7 +274,7 @@ public function payroll(Request $request): JsonResponse // REPT-08: Attendance public function attendance(Request $request): JsonResponse { - if ($request->user()->isEmployee()) { + if (! $this->canViewOthers($request)) { return response()->json(['message' => 'Unauthorized.'], 403); } @@ -251,11 +304,7 @@ public function activityByDay(Request $request): JsonResponse ]); $user = $request->user(); - $userId = $request->user_id; - - if ($user->isEmployee()) { - $userId = $user->id; - } + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); @@ -280,11 +329,7 @@ public function timeLogs(Request $request): JsonResponse ]); $user = $request->user(); - $userId = $request->user_id; - - if ($user->isEmployee()) { - $userId = $user->id; - } + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); @@ -309,11 +354,7 @@ public function analytics(Request $request): JsonResponse ]); $user = $request->user(); - $userId = $request->user_id; - - if ($user->isEmployee()) { - $userId = $user->id; - } + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); @@ -338,11 +379,7 @@ public function detailedLogs(Request $request): JsonResponse ]); $user = $request->user(); - $userId = $request->user_id; - - if ($user->isEmployee()) { - $userId = $user->id; - } + $userId = $this->scopedUserId($request); [$dateFrom, $dateTo] = $this->parseDateRange($request); diff --git a/backend/app/Http/Controllers/Api/V1/TimeEntryController.php b/backend/app/Http/Controllers/Api/V1/TimeEntryController.php index c4e88f02..05a23fab 100644 --- a/backend/app/Http/Controllers/Api/V1/TimeEntryController.php +++ b/backend/app/Http/Controllers/Api/V1/TimeEntryController.php @@ -9,6 +9,7 @@ use App\Services\ManualTimeEntryService; use App\Services\ReportService; use App\Support\TimezoneAwareDateRange; +use App\Support\WorkedTime; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -51,11 +52,39 @@ public function index(Request $request): JsonResponse $query->where('is_approved', filter_var($request->is_approved, FILTER_VALIDATE_BOOLEAN)); } + // Totals for the WHOLE filtered set, computed server-side. + // + // The client used to sum the entries it had been handed, which is one page — + // so the "Total Hours" card reported 20 entries' worth of time next to an + // "Entries" count covering every match, and the total changed as you paged. + // Measured with the canonical expression so this card agrees with the + // dashboard and the reports tab instead of trusting `duration_seconds`. + $dur = WorkedTime::durationExpr('time_entries'); + $totals = (clone $query) + ->withoutEagerLoads() + ->reorder() + ->selectRaw(" + COALESCE(SUM(CASE WHEN time_entries.type <> 'idle' + AND time_entries.approval_status = 'approved' + AND time_entries.ended_at IS NOT NULL THEN {$dur} ELSE 0 END), 0) as worked_seconds, + COALESCE(SUM(CASE WHEN time_entries.type = 'idle' + AND time_entries.ended_at IS NOT NULL THEN {$dur} ELSE 0 END), 0) as idle_seconds, + COALESCE(SUM(CASE WHEN time_entries.type <> 'idle' + AND time_entries.approval_status <> 'approved' + AND time_entries.ended_at IS NOT NULL THEN {$dur} ELSE 0 END), 0) as unapproved_seconds + ") + ->first(); + $entries = $query->orderBy('started_at', 'desc')->paginate( min((int) $request->input('per_page', 25), 100) ); - return response()->json($entries); + $payload = $entries->toArray(); + $payload['total_seconds'] = (int) ($totals->worked_seconds ?? 0); + $payload['idle_seconds'] = (int) ($totals->idle_seconds ?? 0); + $payload['unapproved_seconds'] = (int) ($totals->unapproved_seconds ?? 0); + + return response()->json($payload); } // TIME-06: Show single entry diff --git a/backend/app/Services/ReportService.php b/backend/app/Services/ReportService.php index fe471b9a..bab1c3ad 100644 --- a/backend/app/Services/ReportService.php +++ b/backend/app/Services/ReportService.php @@ -4,7 +4,9 @@ use App\Models\TimeEntry; use App\Models\ActivityLog; +use App\Models\Organization; use App\Models\User; +use App\Support\WorkedTime; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; @@ -15,29 +17,54 @@ class ReportService * Any entry exceeding this is capped to prevent runaway timers from * corrupting report totals. 12 hours = 43200 seconds. */ - private const MAX_ENTRY_DURATION = 43200; + private const MAX_ENTRY_DURATION = WorkedTime::MAX_ENTRY_DURATION; + + /** @var array memoized org timezones */ + private array $orgTimezones = []; /** - * SQL expression for capped duration: compute from timestamps (more accurate - * than duration_seconds which can be corrupted), then cap at MAX_ENTRY_DURATION. + * Canonical entry duration. Delegates to {@see WorkedTime} so the dashboard, + * the reports tab, the report builder and the time list all measure an entry + * the same way. */ private static function durationExpr(string $prefix = ''): string { - $cap = self::MAX_ENTRY_DURATION; - $startCol = $prefix ? "{$prefix}.started_at" : 'started_at'; - $endCol = $prefix ? "{$prefix}.ended_at" : 'ended_at'; + return WorkedTime::durationExpr($prefix); + } + + /** + * The organization's timezone — the zone its days are bucketed by. Daily rollups + * must group by this, not by the UTC date, or work done between local midnight + * and the UTC offset lands on the previous day's row. + */ + private function orgTimezone(string $orgId): string + { + return $this->orgTimezones[$orgId] ??= ( + Organization::withoutGlobalScopes()->find($orgId)?->getSetting('timezone') ?: config('app.timezone', 'UTC') + ); + } - if (DB::connection()->getDriverName() === 'sqlite') { - // SQLite: julianday diff * 86400 = seconds; MIN/MAX are scalar in SQLite - return "MIN(MAX(CAST((julianday({$endCol}) - julianday({$startCol})) * 86400 AS INTEGER), 0), {$cap})"; + /** + * Apply the actor's user filter to a query. + * + * Accepts a single id, a LIST of ids (a `project`-scoped actor may see their + * team, which is more than one person and fewer than the org), or null for + * "everyone in the organization". + */ + private static function applyUserFilter($query, string $column, string|array|null $userId) + { + if ($userId === null || $userId === []) { + return $query; } - return "LEAST(GREATEST(EXTRACT(EPOCH FROM ({$endCol} - {$startCol}))::int, 0), {$cap})"; + return is_array($userId) + ? $query->whereIn($column, $userId) + : $query->where($column, $userId); } - private function cacheKey(string $orgId, string $type, string $period, ?string $userId = null): string + private function cacheKey(string $orgId, string $type, string $period, string|array|null $userId = null): string { - $userHash = $userId ? md5($userId) : 'all'; + $userHash = $userId ? md5(is_array($userId) ? implode(',', $userId) : $userId) : 'all'; // Embed the org's cache generation so flushForOrg() can invalidate every // cached report for an org in one shot (store-agnostic — the default // database cache driver cannot flush by tag). @@ -76,7 +103,7 @@ public function flushForOrg(string $orgId): void } // REPT-01: Summary report - public function summary(string $orgId, ?string $userId, string $dateFrom, string $dateTo): array + public function summary(string $orgId, string|array|null $userId, string $dateFrom, string $dateTo): array { $cacheKey = $this->cacheKey($orgId, 'summary', "{$dateFrom}_{$dateTo}", $userId); @@ -92,17 +119,20 @@ public function summary(string $orgId, ?string $userId, string $dateFrom, string // so tracked/idle and every historical row are unaffected. ->where('approval_status', 'approved'); - if ($userId) { - $query->where('user_id', $userId); - } + self::applyUserFilter($query, 'user_id', $userId); $dur = self::durationExpr(); + // Bucket by the ORGANIZATION's calendar day. `started_at` holds UTC, so a + // bare DATE() would push work done between local midnight and the UTC + // offset onto the previous day's row — the daily series then disagrees + // with the dashboard's weekly chart, which already buckets in org time. + $localDate = WorkedTime::localDateExpr($this->orgTimezone($orgId)); // Use EXTRACT(EPOCH FROM ...) for accurate duration — duration_seconds // can be corrupted by idle-deduction bugs in older desktop versions. // Cap each entry at MAX_ENTRY_DURATION to prevent runaway timers. $daily = $query->selectRaw(" - DATE(started_at) as date, + {$localDate} as date, SUM({$dur}) as total_seconds, CASE WHEN SUM(CASE WHEN activity_score IS NOT NULL AND activity_score > 0 @@ -117,7 +147,7 @@ public function summary(string $orgId, ?string $userId, string $dateFrom, string COALESCE(SUM(CASE WHEN type <> 'idle' THEN {$dur} ELSE 0 END), 0) as worked_seconds, COALESCE(SUM(CASE WHEN type = 'idle' THEN {$dur} ELSE 0 END), 0) as idle_seconds ") - ->groupBy(DB::raw('DATE(started_at)')) + ->groupBy(DB::raw($localDate)) ->orderBy('date') ->get(); @@ -149,7 +179,7 @@ public function summary(string $orgId, ?string $userId, string $dateFrom, string ->where('projects.billable', true); if ($userId) { - $billableQuery->where('time_entries.user_id', $userId); + self::applyUserFilter($billableQuery, 'time_entries.user_id', $userId); } $billableResult = $billableQuery @@ -184,7 +214,7 @@ public function summary(string $orgId, ?string $userId, string $dateFrom, string ->where('approval_status', 'approved'); if ($userId) { - $prevQuery->where('user_id', $userId); + self::applyUserFilter($prevQuery, 'user_id', $userId); } $prevTotalSeconds = (int) ($prevQuery->selectRaw("COALESCE(SUM({$dur}), 0) as total_seconds")->value('total_seconds') ?? 0); @@ -205,7 +235,7 @@ public function summary(string $orgId, ?string $userId, string $dateFrom, string ->where('projects.billable', true); if ($userId) { - $prevBudgetQuery->where('time_entries.user_id', $userId); + self::applyUserFilter($prevBudgetQuery, 'time_entries.user_id', $userId); } $previousBudgetUsed = (float) ($prevBudgetQuery @@ -352,7 +382,7 @@ public function projects(string $orgId, string $dateFrom, string $dateTo): array // REPT-04: App usage — per-user per-app when no user filter; per-app for one user. // Uses app_usage_summaries (calendar dates) instead of raw activity_logs. - public function apps(string $orgId, ?string $userId, string $dateFrom, string $dateTo): array + public function apps(string $orgId, string|array|null $userId, string $dateFrom, string $dateTo): array { $cacheKey = $this->cacheKey($orgId, 'apps', "{$dateFrom}_{$dateTo}", $userId); @@ -364,7 +394,7 @@ public function apps(string $orgId, ?string $userId, string $dateFrom, string $d ->whereBetween('app_usage_summaries.date', [$dateFrom, $dateTo]); if ($userId) { - $query->where('app_usage_summaries.user_id', $userId); + self::applyUserFilter($query, 'app_usage_summaries.user_id', $userId); } $select = [ @@ -474,7 +504,7 @@ public function payroll(string $orgId, string $dateFrom, string $dateTo): array } // REPT-09: Analytics (KPIs + chart data) - public function analytics(string $orgId, ?string $userId, string $dateFrom, string $dateTo): array + public function analytics(string $orgId, string|array|null $userId, string $dateFrom, string $dateTo): array { $cacheKey = $this->cacheKey($orgId, 'analytics', "{$dateFrom}_{$dateTo}", $userId); @@ -489,7 +519,7 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri ->whereNotNull('te.ended_at') ->where('te.approval_status', 'approved'); if ($userId) { - $q->where('te.user_id', $userId); + self::applyUserFilter($q, 'te.user_id', $userId); } }; @@ -498,10 +528,14 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri $prevFrom = date('Y-m-d H:i:s', strtotime($dateFrom) - $periodLengthSeconds); $prevTo = $dateFrom; + // Worked hours = every approved entry that is NOT idle. Filtering to + // type='tracked' here made this KPI the only headline in the product that + // silently dropped approved MANUAL time, so the reports tab read lower than + // the report builder and the dashboard over the very same range. $currentHoursRow = DB::table('time_entries as te') ->whereNull('te.deleted_at') ->where(function ($q) use ($baseWhere) { $baseWhere($q); }) - ->where('te.type', 'tracked') + ->where('te.type', '<>', 'idle') ->selectRaw("COALESCE(SUM({$dur}), 0) as total_seconds") ->first(); $currentTotalSeconds = (int) ($currentHoursRow->total_seconds ?? 0); @@ -513,9 +547,9 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri ->where('te.started_at', '>=', $prevFrom) ->where('te.started_at', '<', $prevTo) ->whereNotNull('te.ended_at') - ->where('te.type', 'tracked') + ->where('te.type', '<>', 'idle') ->where('te.approval_status', 'approved') - ->when($userId, fn ($q) => $q->where('te.user_id', $userId)) + ->when($userId, fn ($q) => self::applyUserFilter($q, 'te.user_id', $userId)) ->selectRaw("COALESCE(SUM({$dur}), 0) as total_seconds") ->first(); $prevTotalSeconds = (int) ($prevHoursRow->total_seconds ?? 0); @@ -547,6 +581,7 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri ->whereNull('te.deleted_at') ->join('projects as p', 'te.project_id', '=', 'p.id') ->where(function ($q) use ($baseWhere) { $baseWhere($q); }) + ->where('te.type', '<>', 'idle') ->where('p.billable', true) ->selectRaw("COALESCE(SUM({$dur} / 3600.0 * p.hourly_rate), 0) as total_budget") ->first(); @@ -560,8 +595,9 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri ->where('te.started_at', '<', $prevTo) ->whereNotNull('te.ended_at') ->where('te.approval_status', 'approved') + ->where('te.type', '<>', 'idle') ->where('p.billable', true) - ->when($userId, fn ($q) => $q->where('te.user_id', $userId)) + ->when($userId, fn ($q) => self::applyUserFilter($q, 'te.user_id', $userId)) ->selectRaw("COALESCE(SUM({$dur} / 3600.0 * p.hourly_rate), 0) as total_budget") ->first(); $prevBudget = (float) ($prevBudgetRow->total_budget ?? 0); @@ -587,10 +623,13 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri $billablePercent = $totalBillableSec > 0 ? (int) round($billableSec / $totalBillableSec * 100) : 0; // --- Chart 1: time_per_project (top 8) --- + // Excludes idle, so the bars sum to the "total hours" KPI above them. + // With no type filter this chart counted idle time the KPI did not. $timePerProject = DB::table('time_entries as te') ->whereNull('te.deleted_at') ->join('projects as p', 'te.project_id', '=', 'p.id') ->where(function ($q) use ($baseWhere) { $baseWhere($q); }) + ->where('te.type', '<>', 'idle') ->whereNotNull('te.project_id') ->selectRaw("p.name as project_name, p.color, SUM({$dur}) / 3600.0 as total_hours") ->groupBy('p.id', 'p.name', 'p.color') @@ -657,7 +696,7 @@ public function analytics(string $orgId, ?string $userId, string $dateFrom, stri } // REPT-10: Detailed logs (paginated time entries with joins) - public function detailedLogs(string $orgId, ?string $userId, string $dateFrom, string $dateTo, int $perPage = 10, int $page = 1): array + public function detailedLogs(string $orgId, string|array|null $userId, string $dateFrom, string $dateTo, int $perPage = 10, int $page = 1): array { $cacheKey = $this->cacheKey($orgId, 'detailed_logs', "{$dateFrom}_{$dateTo}_{$perPage}_{$page}", $userId); @@ -673,10 +712,13 @@ public function detailedLogs(string $orgId, ?string $userId, string $dateFrom, s ->where('te.started_at', '>=', $dateFrom) ->where('te.started_at', '<', $dateTo) ->whereNotNull('te.ended_at') + // Idle is not worked time; listing it here made the rows fail to + // reconcile with the totals rendered above the table. + ->where('te.type', '<>', 'idle') ->where('te.approval_status', 'approved'); if ($userId) { - $baseQuery->where('te.user_id', $userId); + self::applyUserFilter($baseQuery, 'te.user_id', $userId); } $total = (clone $baseQuery)->count(); @@ -730,7 +772,7 @@ public function detailedLogs(string $orgId, ?string $userId, string $dateFrom, s } // REPT-11: Activity by day of week (weighted average activity per weekday) - public function activityByDay(string $orgId, ?string $userId, string $dateFrom, string $dateTo): array + public function activityByDay(string $orgId, string|array|null $userId, string $dateFrom, string $dateTo): array { $cacheKey = $this->cacheKey($orgId, 'activity_by_day', "{$dateFrom}_{$dateTo}", $userId); @@ -750,7 +792,7 @@ public function activityByDay(string $orgId, ?string $userId, string $dateFrom, ->where('activity_score', '>', 0); if ($userId) { - $query->where('user_id', $userId); + self::applyUserFilter($query, 'user_id', $userId); } $rows = $query->selectRaw(" @@ -783,7 +825,7 @@ public function activityByDay(string $orgId, ?string $userId, string $dateFrom, } // REPT-12: Detailed time logs (paginated, with user/project/task joins) - public function timeLogs(string $orgId, ?string $userId, string $dateFrom, string $dateTo, int $perPage = 15): \Illuminate\Contracts\Pagination\LengthAwarePaginator + public function timeLogs(string $orgId, string|array|null $userId, string $dateFrom, string $dateTo, int $perPage = 15): \Illuminate\Contracts\Pagination\LengthAwarePaginator { $dur = self::durationExpr('time_entries'); @@ -796,10 +838,11 @@ public function timeLogs(string $orgId, ?string $userId, string $dateFrom, strin ->where('time_entries.started_at', '>=', $dateFrom) ->where('time_entries.started_at', '<', $dateTo) ->whereNotNull('time_entries.ended_at') + ->where('time_entries.type', '<>', 'idle') ->where('time_entries.approval_status', 'approved'); if ($userId) { - $query->where('time_entries.user_id', $userId); + self::applyUserFilter($query, 'time_entries.user_id', $userId); } return $query->select([ diff --git a/backend/app/Support/WorkedTime.php b/backend/app/Support/WorkedTime.php new file mode 100644 index 00000000..ad5991e2 --- /dev/null +++ b/backend/app/Support/WorkedTime.php @@ -0,0 +1,91 @@ + 'idle'`. + * - The daily series grouped by DATE(started_at), i.e. the UTC date, while the + * range bounds around it were resolved in the organization timezone. + */ +class WorkedTime +{ + /** + * Hard ceiling for a single entry. A runaway timer (a session nobody stopped, + * a dead machine) must not be able to bill days of wall-clock into a rollup. + */ + public const MAX_ENTRY_DURATION = 43200; // 12h + + /** + * Canonical duration of one entry, in seconds. + * + * Derived from the timestamps rather than read from `duration_seconds`, then + * clamped to [0, MAX_ENTRY_DURATION] so corrupt rows can neither subtract time + * nor inflate it. + */ + public static function durationExpr(string $prefix = ''): string + { + $cap = self::MAX_ENTRY_DURATION; + $startCol = $prefix ? "{$prefix}.started_at" : 'started_at'; + $endCol = $prefix ? "{$prefix}.ended_at" : 'ended_at'; + + if (DB::connection()->getDriverName() === 'sqlite') { + // SQLite: julianday diff * 86400 = seconds; MIN/MAX are scalar here. + return "MIN(MAX(CAST((julianday({$endCol}) - julianday({$startCol})) * 86400 AS INTEGER), 0), {$cap})"; + } + + return "LEAST(GREATEST(EXTRACT(EPOCH FROM ({$endCol} - {$startCol}))::int, 0), {$cap})"; + } + + /** + * The calendar date an entry belongs to, in the organization's timezone. + * + * `started_at` is a `timestamp without time zone` holding UTC, so DATE() on it + * yields the UTC date. For Asia/Karachi (UTC+5) that pushes everything worked + * between local 00:00 and 05:00 onto the previous day's bar. + */ + public static function localDateExpr(string $timezone, string $prefix = ''): string + { + $startCol = $prefix ? "{$prefix}.started_at" : 'started_at'; + + if (DB::connection()->getDriverName() === 'sqlite') { + return "DATE({$startCol})"; + } + + return "DATE({$startCol} AT TIME ZONE 'UTC' AT TIME ZONE " . DB::getPdo()->quote($timezone) . ")"; + } + + /** + * Restrict a query to the entries that count as WORKED time. + * + * Worked = every APPROVED entry that is not idle. "Tracked" is a row type, not a + * synonym for "worked": filtering on it drops approved manual time, which is how + * the reports tab came to under-report by the whole manual bucket. + * + * @param EloquentBuilder|QueryBuilder $query + */ + public static function scopeWorked($query, string $prefix = '') + { + $col = fn (string $c) => $prefix ? "{$prefix}.{$c}" : $c; + + return $query + ->whereNotNull($col('ended_at')) + ->where($col('type'), '<>', 'idle') + ->where($col('approval_status'), 'approved'); + } +} diff --git a/backend/tests/Feature/Timer/WorkedTimeConsistencyTest.php b/backend/tests/Feature/Timer/WorkedTimeConsistencyTest.php new file mode 100644 index 00000000..887e106a --- /dev/null +++ b/backend/tests/Feature/Timer/WorkedTimeConsistencyTest.php @@ -0,0 +1,247 @@ +org = $this->createOrganization(); + $this->owner = $this->createUser($this->org, 'owner'); + $this->employee = $this->createUser($this->org, 'employee'); + } + + protected function tearDown(): void + { + Carbon::setTestNow(); + parent::tearDown(); + } + + private function entry(string $type, string $start, int $seconds, string $status = 'approved'): TimeEntry + { + $startedAt = Carbon::parse(self::DAY . ' ' . $start, 'UTC'); + + return TimeEntry::factory()->create([ + 'organization_id' => $this->org->id, + 'user_id' => $this->employee->id, + 'type' => $type, + 'started_at' => $startedAt, + 'ended_at' => $startedAt->copy()->addSeconds($seconds), + 'duration_seconds' => $seconds, + 'approval_status' => $status, + 'is_approved' => $status === 'approved', + ]); + } + + /** tracked 2h + approved manual 1h + idle 30m. Worked = 3h. */ + private function baseline(): void + { + $this->entry('tracked', '08:00:00', self::TRACKED_SECONDS); + $this->entry('manual', '11:00:00', self::MANUAL_SECONDS); + $this->entry('idle', '13:00:00', self::IDLE_SECONDS); + } + + public function test_every_surface_reports_the_same_worked_total(): void + { + $this->baseline(); + $this->actingAs($this->owner, 'sanctum'); + + $expectedHours = round(self::WORKED_SECONDS / 3600, 1); // 3.0 + + // 1. Report builder / summary + $summary = app(ReportService::class)->summary( + $this->org->id, null, self::FROM . ' 00:00:00', self::TO . ' 00:00:00' + ); + $this->assertSame(self::WORKED_SECONDS, (int) $summary['total_seconds_worked'], 'report builder'); + + // 2. Reports tab KPI — used to filter type='tracked' and drop the manual hour + $analytics = app(ReportService::class)->analytics( + $this->org->id, null, self::FROM . ' 00:00:00', self::TO . ' 00:00:00' + ); + $this->assertSame( + $expectedHours, + $analytics['kpis']['total_tracked_hours']['value'], + 'reports tab KPI must count approved manual time and exclude idle' + ); + + // 3. Dashboard (the per-user card lives on the employee dashboard) + $this->actingAs($this->employee, 'sanctum'); + $dashboard = $this->getJson('/api/v1/dashboard?date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + $this->assertSame(self::WORKED_SECONDS, (int) $dashboard['today_seconds'], 'dashboard'); + + // 4. Time page card + $entries = $this->getJson('/api/v1/time-entries?date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + $this->assertSame(self::WORKED_SECONDS, (int) $entries['total_seconds'], 'time page card'); + + // 5. Time series — the daily rows must sum to the same headline + $this->assertSame( + self::WORKED_SECONDS, + (int) collect($summary['daily'])->sum('worked_seconds'), + 'daily series must reconcile with the headline total' + ); + } + + public function test_idle_time_is_never_counted_as_worked_time(): void + { + $this->baseline(); + $this->actingAs($this->owner, 'sanctum'); + + $entries = $this->getJson('/api/v1/time-entries?date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + + $this->assertSame(self::WORKED_SECONDS, (int) $entries['total_seconds']); + $this->assertSame(self::IDLE_SECONDS, (int) $entries['idle_seconds'], 'idle is reported separately, not folded in'); + } + + public function test_a_corrupt_duration_seconds_column_cannot_move_any_total(): void + { + $this->baseline(); + + // Exactly the corruption present in real data: a negative stored duration, + // which a SUM(duration_seconds) subtracts from the day's total. + DB::table('time_entries') + ->where('organization_id', $this->org->id) + ->where('type', 'tracked') + ->update(['duration_seconds' => -9999]); + + $summary = app(ReportService::class)->summary( + $this->org->id, null, self::FROM . ' 00:00:00', self::TO . ' 00:00:00' + ); + $this->assertSame(self::WORKED_SECONDS, (int) $summary['total_seconds_worked']); + + $this->actingAs($this->employee, 'sanctum'); + $dashboard = $this->getJson('/api/v1/dashboard?date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + $this->assertSame(self::WORKED_SECONDS, (int) $dashboard['today_seconds']); + + $entries = $this->getJson('/api/v1/time-entries?date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + $this->assertSame(self::WORKED_SECONDS, (int) $entries['total_seconds']); + } + + public function test_time_entry_total_covers_the_whole_filtered_set_not_one_page(): void + { + // 25 half-hour entries = 12.5h, deliberately more than one page of 10. + for ($i = 0; $i < 25; $i++) { + $startedAt = Carbon::parse(self::DAY . ' 00:00:00', 'UTC')->addMinutes($i * 31); + TimeEntry::factory()->create([ + 'organization_id' => $this->org->id, + 'user_id' => $this->employee->id, + 'type' => 'tracked', + 'started_at' => $startedAt, + 'ended_at' => $startedAt->copy()->addMinutes(30), + 'duration_seconds' => 1800, + 'approval_status' => 'approved', + 'is_approved' => true, + ]); + } + + $this->actingAs($this->owner, 'sanctum'); + + $expected = 25 * 1800; + + $page1 = $this->getJson('/api/v1/time-entries?per_page=10&page=1&date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + $page3 = $this->getJson('/api/v1/time-entries?per_page=10&page=3&date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + + $this->assertCount(10, $page1['data']); + $this->assertSame(25, (int) $page1['total'], 'entry count covers the filtered set'); + $this->assertSame($expected, (int) $page1['total_seconds'], 'total must cover all 25, not the 10 on screen'); + + // The headline must not move as the user pages. + $this->assertSame( + (int) $page1['total_seconds'], + (int) $page3['total_seconds'], + 'the total changed between pages' + ); + } + + public function test_pending_manual_time_is_excluded_from_the_time_page_total(): void + { + $this->entry('tracked', '08:00:00', self::TRACKED_SECONDS); + $this->entry('manual', '11:00:00', self::MANUAL_SECONDS, 'pending'); + + $this->actingAs($this->owner, 'sanctum'); + + $entries = $this->getJson('/api/v1/time-entries?date_from=' . self::FROM . '&date_to=' . self::TO) + ->assertOk()->json(); + + $this->assertSame(self::TRACKED_SECONDS, (int) $entries['total_seconds']); + $this->assertSame(self::MANUAL_SECONDS, (int) $entries['unapproved_seconds']); + } + + public function test_daily_series_buckets_by_the_organization_timezone_not_utc(): void + { + // Asia/Karachi is UTC+5. 2026-06-17 21:00 UTC is 2026-06-18 02:00 locally, + // so this hour belongs to the 18th. Grouping on the raw UTC date filed it + // under the 17th, which is how the series disagreed with the dashboard. + $this->org->settings = array_merge($this->org->settings ?? [], ['timezone' => 'Asia/Karachi']); + $this->org->save(); + + $startedAt = Carbon::parse('2026-06-17 21:00:00', 'UTC'); + TimeEntry::factory()->create([ + 'organization_id' => $this->org->id, + 'user_id' => $this->employee->id, + 'type' => 'tracked', + 'started_at' => $startedAt, + 'ended_at' => $startedAt->copy()->addHour(), + 'duration_seconds' => 3600, + 'approval_status' => 'approved', + 'is_approved' => true, + ]); + + $summary = app(ReportService::class)->summary( + $this->org->id, null, '2026-06-16 00:00:00', '2026-06-19 00:00:00' + ); + + $dates = collect($summary['daily'])->pluck('date')->map(fn ($d) => substr((string) $d, 0, 10))->all(); + + $this->assertContains('2026-06-18', $dates, 'entry must land on its LOCAL day'); + $this->assertNotContains('2026-06-17', $dates, 'entry must not land on the UTC day'); + } +} diff --git a/bugs/README.md b/bugs/README.md index 0e1909cb..8bb48d64 100644 --- a/bugs/README.md +++ b/bugs/README.md @@ -27,6 +27,7 @@ Verify `file:line` references still match the codebase before implementing from | File | Area | Severity | Status / symptom | | ------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| [web-time-totals-disagree-across-surfaces.md](web-time-totals-disagree-across-surfaces.md) | Web reporting + dashboard | P1 | ✅ FIXED (2026-08-21) — the same date range produced four different totals: reports tab 23.02h, report builder 31.52h, dashboard 31.51h, Time page 31.68h (of which it showed only the page on screen — 3905h against a true 2001h across the full dataset). No shared definition of "worked time" existed, and the four surfaces disagreed on all three axes at once. **Which rows:** the reports KPI filtered `type='tracked'`, dropping approved MANUAL time, while its own charts and log tables filtered nothing and counted IDLE. **How long a row is:** the dashboard summed `duration_seconds`, a desktop-written column holding 1897h of phantom idle against 55h real, plus rows storing NEGATIVE seconds that a `SUM()` subtracts. **Which day:** `summary()` grouped by `DATE(started_at)` — the UTC date — inside range bounds resolved in the org timezone, so Asia/Karachi work before 05:00 local landed on the previous day's bar. Fix: `App\Support\WorkedTime` is now the one definition (clamped timestamp-derived duration, org-timezone day bucket, approved-non-idle scope), wired through ReportService, DashboardController and TimeEntryController, which now returns the total for the WHOLE filtered set instead of letting the client sum a page | | [desktop-no-notification-on-return-from-break.md](desktop-no-notification-on-return-from-break.md) | Desktop notifications + idle watchdog | P1 | ✅ FIXED (2026-08-20) — employees came back from a break, saw nothing, and kept working untracked after the idle watchdog had auto-stopped the timer. Needs one ordinary setup to reproduce: a machine that never sleeps and never locks (charger + external display), where **every** existing path misses the user. The idle alert beeps into an empty room and is then dismissed by the stop; the "Timer auto-stopped" toast fires at the moment of the stop, hours before anyone returns; `notifyTrackingState()` — the function whose entire job is telling the user their state — only runs on `wake`/`unlock`/`startup`, and such a machine emits none of those; and the idle watchdog self-gates on `isTimerRunning`, so the instant it stops the timer it stops looking. Nothing watched for the user coming BACK. Fix: a return-from-break watcher — the mirror of the watchdog, running while the timer is STOPPED — polling `getSystemIdleTime()` every 15s and firing when the user has been away past the org idle threshold and is active again. Absence is **peak-tracked**, because the poll that catches the return reads 0 (the OS counter resets on the first keystroke) and a last-reading model would make every absence invisible; a `NaN` reading fails silent rather than inventing an absence. Three cues, since each alone is routinely swallowed: a `silent:false` toast with a unique id (Action Center would otherwise dedup it against the old auto-stop toast), an in-renderer WebAudio beep (macOS Focus / Windows Action Center drop notification sound — three descending tones, distinct from the idle alert's two rising ones), and the window itself with a red banner that clears when tracking resumes | | [desktop-screenshots-bound-to-local-entry-id.md](desktop-screenshots-bound-to-local-entry-id.md) | Desktop screenshot capture + offline queue + session purge | P1 | ✅ FIXED (2026-08-20) — screenshots stopped reaching the dashboard while time sync stayed healthy: on 2026-08-20 seventeen users tracked 5–8h each and three had any screenshot at all. Live uploads (arrival within 60s of capture) went **1627/1639 on Aug 12 → 0/835 on Aug 18** — the offline-first rollout landed Aug 13. `startTimer()` makes no network call any more, so `currentEntry.id` is the local SQLite id `local--`, and it was passed straight to `screenshotService.start()`; `POST /screenshots/presign` validates `time_entry_id` as a **uuid** and looks it up in `time_entries`, so every live shot 422'd and fell into the offline queue. `rebindEntryId()` — written for exactly this — survived the refactor but its only caller, `reconcileTimerState()`, was deleted with it. The queue then lost them for good: resolution reads `timer_sessions.server_entry_id`, but the 05:00 purge (24h) and `clearForLogout()` delete that row with no knowledge of the queue's separate DB, and with a **15–42 hour** backlog lag shots routinely outlived their own session; the `idempotency_key` fallback could not help because the queued key is the SHOT's dedupe key, never the session's. Unresolvable items are *held*, not dropped, so they piled up behind `LIMIT 500` and were finally deleted **silently, with their image files**, by the 7-day TTL sweep. Heartbeats were unaffected (priority 1, and they carry the session uuid) — which is what isolated the fault. Fix: `onSessionConfirmed` rebinds capture to the real entry id the moment a session first syncs; `liveCaptureEntryId()` seeds `start()` from an already-known id; queued shots record `session_uuid` and resolve through it; `purgeConfirmed()` takes a keep-list fed by `offlineQueue.referencedSessionKeys()`; held shots are now logged instead of vanishing | | [approved-manual-time-missing-from-dashboard-and-reports.md](approved-manual-time-missing-from-dashboard-and-reports.md) | Backend dashboard + timer totals + reports, web reports page | P1 | ✅ FIXED (2026-08-18) — a manual entry showed **Approved** on Time Entries and counted in the day's page total (7:17:34), but Today's Hours read 7h 5m and the Reports summary read 5:31:35 while listing **5 entries** — it counted the row and dropped its 13 minutes. Two separate filters, one mistake: `DashboardController` (team/today/week/daily/active-projects) and `TimerService` (`status()`, `todayTotal()`) narrowed to `type='tracked'`, and the reports page/CSV/PDF rendered `total_seconds_tracked` beside a `COUNT(*)` taken over the whole approved set. Every other rollup — ReportService, timesheets, AttendanceService, DailyActivitySummaryService — already counted approved manual time, and `todayTotal` is documented as the one place the desktop picks manual entries up. All of them now apply one rule: worked time = every APPROVED entry that is not idle. `ReportService` gained `worked_seconds`/`total_seconds_worked`; `tracked_seconds` stays narrow because `idle_percent` and the billable gate are defined against it | diff --git a/bugs/web-time-totals-disagree-across-surfaces.md b/bugs/web-time-totals-disagree-across-surfaces.md new file mode 100644 index 00000000..5d9931c1 --- /dev/null +++ b/bugs/web-time-totals-disagree-across-surfaces.md @@ -0,0 +1,92 @@ +# Web reports four different totals for the same time range + +**Area:** Web dashboard + Laravel reporting layer +**Severity:** P1 — the product cannot state how long anyone worked +**Status:** ✅ FIXED (2026-08-21, `develop`) + +## Symptom + +Four surfaces, one date range, four answers. Measured on the dev dataset over an +identical 60-day window, org-wide: + +| Surface | Before | After | +| --- | --- | --- | +| Reports tab KPI | 23.02 h | 31.52 h | +| Report builder (summary) | 31.52 h | 31.52 h | +| Dashboard | 31.51 h | 31.52 h | +| Time page "Total Hours" | 31.68 h *(and it only displayed one page's slice)* | 31.52 h | + +Over the full dataset for the main org the Time page card was the worst offender: +**3905 h against a true 2001 h.** + +## Root cause + +There was no shared definition of "worked time". Each surface had grown its own, +and the four disagreed on all three axes at once — which rows count, how long a row +is, and which day it belongs to. + +### 1. Which rows count + +`ReportService::analytics()` filtered `type = 'tracked'` for its headline KPI, so every +approved **manual** entry was invisible there while the report builder, dashboard, +timesheets and attendance all counted it. This is the exact rule CLAUDE.md already +states — *worked time = every APPROVED entry that is NOT idle, never `type='tracked'`* — +re-broken in a surface the earlier fix did not cover. + +The same method's charts had the opposite defect: `time_per_project` and the budget +KPI filtered **nothing**, so they counted **idle** time the KPI above them excluded. +`detailedLogs()` and `timeLogs()` likewise listed idle rows under totals that omitted +them, so the table never reconciled with its own header. + +### 2. How long a row is + +`DashboardController` summed the `duration_seconds` column; the reporting layer derived +duration from the timestamps and clamped it. `duration_seconds` is written by the desktop +agent and is not trustworthy — on the dev dataset: + +- idle rows store **1897 h** against **55.65 h** of real wall-clock (4683 of 4889 rows wrong); +- 69 tracked rows disagree with their own timestamps, several storing **negative** seconds + (`-1163`, `-613`, `-478`, `-370`), which `SUM(duration_seconds)` silently **subtracts**. + +That is why the Time page — summing `duration_seconds` across all types, idle included — +nearly doubled the true figure. + +### 3. Which day a row belongs to + +`summary()` grouped by `DATE(started_at)`. `started_at` is `timestamp without time zone` +holding UTC, so that is the **UTC** date, while the range bounds around it were resolved in +the organization timezone. For Asia/Karachi (UTC+5) everything worked between local 00:00 +and 05:00 landed on the previous day's bar — 28 entries in the current dataset. The +dashboard's weekly chart already bucketed in org time, so the two series drew different days. + +### 4. The Time page summed the page + +`totalSeconds` was `entries.reduce(...)` over the fetched page — 20 rows — rendered beside +an "Entries" count covering the whole filtered set, and it changed as the user paged. + +## Fix + +`backend/app/Support/WorkedTime.php` (new) is now the single definition: + +- `durationExpr()` — duration from timestamps, clamped to `[0, 43200]`, so a corrupt row can + neither subtract time nor inflate it; +- `localDateExpr($tz)` — the calendar day in the **organization** timezone; +- `scopeWorked()` — approved, `type <> 'idle'`, ended. + +Wired through `ReportService` (summary bucketing, analytics KPI + charts, detailed logs, +time logs), `DashboardController` (all four rollups), and `TimeEntryController::index()`, +which now returns `total_seconds` / `idle_seconds` / `unapproved_seconds` computed +server-side over the **whole filtered set**. `web/.../time/page.tsx` renders that total +instead of summing the page; the reports KPI is relabelled "Total Hours" to match what it +now measures. + +## Regression cover + +`backend/tests/Feature/Timer/WorkedTimeConsistencyTest.php` — asserts all five surfaces +return the same total for one fixture, that idle never counts as worked, that a **negative** +`duration_seconds` cannot move any total, that the Time page total covers the filtered set +and does not change between pages, that pending manual time stays out, and that the daily +series buckets on the local day rather than the UTC one. + +Verified failing against the pre-fix code (KPI returned 2.0 h where 3.0 h was correct; the +daily series filed the entry under the UTC day). diff --git a/web/src/app/(dashboard)/reports/page.tsx b/web/src/app/(dashboard)/reports/page.tsx index 5331a875..dbe93bee 100644 --- a/web/src/app/(dashboard)/reports/page.tsx +++ b/web/src/app/(dashboard)/reports/page.tsx @@ -49,12 +49,7 @@ import { usePermissionStore } from "@/stores/permission-store"; type DatePreset = "today" | "7days" | "month" | "custom"; type ReportType = - | "summary" - | "team" - | "projects" - | "apps" - | "payroll" - | "attendance"; + "summary" | "team" | "projects" | "apps" | "payroll" | "attendance"; interface AnalyticsData { total_hours: number; @@ -200,7 +195,10 @@ function transformReportResponse( rows: daily.map((d) => ({ date: String(d.date ?? ""), tracked_seconds: Number( - d.worked_seconds ?? d.tracked_seconds ?? d.total_seconds ?? 0, + d.worked_seconds ?? + d.tracked_seconds ?? + d.total_seconds ?? + 0, ), idle_seconds: Number(d.idle_seconds ?? 0), activity_score_avg: Number(d.activity_score_avg ?? 0), @@ -392,8 +390,14 @@ const reportTypes: { value: ReportType; label: string; description: string }[] = export default function ReportsPage() { const router = useRouter(); const { user } = useAuthStore(); - const { hasPermission } = usePermissionStore(); + const { hasPermission, hasPermissionWithScope } = usePermissionStore(); const isEmployee = !hasPermission("reports.view"); + // `reports.view` is granted to the employee role at scope 'own', so holding the + // permission does NOT mean you may look at other people. Gating the user picker + // on the bare permission rendered a full team dropdown for own-scoped users and + // the API then clamped every request back to the caller — so whatever you picked, + // including "All Users", you saw your own figures. Same test the Time page uses. + const canViewOthers = hasPermissionWithScope("reports.view", "project"); // Redirect employees useEffect(() => { @@ -511,7 +515,7 @@ export default function ReportsPage() { (Array.isArray(res.data) ? res.data : []) ); }, - enabled: !isEmployee, + enabled: !isEmployee && canViewOthers, }); const { @@ -525,14 +529,14 @@ export default function ReportsPage() { reportType, builderDateFrom, builderDateTo, - userFilter, + canViewOthers ? userFilter : "self", ], queryFn: async () => { const params: Record = { date_from: builderDateFrom, date_to: builderDateTo, }; - if (userFilter && userFilter !== "all") { + if (canViewOthers && userFilter && userFilter !== "all") { params.user_id = userFilter; } const res = await api.get(`/reports/${reportType}`, { params }); @@ -555,7 +559,10 @@ export default function ReportsPage() { date_from: builderDateFrom, date_to: builderDateTo, format: exportFormat, - user_id: userFilter !== "all" ? userFilter : undefined, + user_id: + canViewOthers && userFilter !== "all" + ? userFilter + : undefined, }, { responseType: "blob" }, ); @@ -756,7 +763,10 @@ export default function ReportsPage() {
{ setDateFrom(val); if (val > dateTo) setDateTo(val); }} + onChange={(val) => { + setDateFrom(val); + if (val > dateTo) setDateTo(val); + }} placeholder="Start date" maxDate={dateTo} /> @@ -833,7 +843,7 @@ export default function ReportsPage() { )}

- Total Tracked Hours + Total Hours

{analytics.total_hours.toLocaleString( @@ -951,7 +961,9 @@ export default function ReportsPage() { {/* Left: Time per Project */}

-

Time per Project

+

+ Time per Project +

Distribution across top active projects

@@ -1039,7 +1051,9 @@ export default function ReportsPage() { {/* Right: Team Activity Levels */}
-

Team Activity Levels

+

+ Team Activity Levels +

Daily average engagement percentages

@@ -1129,7 +1143,9 @@ export default function ReportsPage() { {/* ── Section 4: Detailed Time Logs Table ── */}
-

Detailed Time Logs

+

+ Detailed Time Logs +

Individual time entries for the selected period

@@ -1348,10 +1364,12 @@ export default function ReportsPage() { }} >
-

Advanced Report Builder

+

+ Advanced Report Builder +

- Generate custom reports with specific filters - and export options + Generate custom reports with specific filters and + export options

{showReportBuilder ? ( @@ -1409,7 +1427,8 @@ export default function ReportsPage() { value={builderDateFrom} onChange={(val) => { setBuilderDateFrom(val); - if (val > builderDateTo) setBuilderDateTo(val); + if (val > builderDateTo) + setBuilderDateTo(val); setShouldFetch(false); }} placeholder="Start date" @@ -1431,39 +1450,45 @@ export default function ReportsPage() { />
-
- - { + setUserFilter(val ?? "all"); + setShouldFetch(false); + }} + > + + + {userFilter === "all" + ? "All Users" + : (teamUsers?.find( + (u) => + u.id === + userFilter, + )?.name ?? "Select user")} + + + + + All Users - ))} - - -
+ {teamUsers?.map((u) => ( + + {u.name} + + ))} + + +
+ )}