diff --git a/app/Domain/Tickets/Repositories/Tickets.php b/app/Domain/Tickets/Repositories/Tickets.php index 207ff8727..506e5fd0a 100644 --- a/app/Domain/Tickets/Repositories/Tickets.php +++ b/app/Domain/Tickets/Repositories/Tickets.php @@ -1902,6 +1902,69 @@ public function getStatusChangeEvents(array $ticketIds, string $fromDate, string return $events; } + /** + * Distinct ticket ids the given user COMMENTED on within [from, to] + * (inclusive, 'Y-m-d'). Access-agnostic on its own — callers must constrain + * the result to an access-scoped set (e.g. the user's accessible projects) + * before returning anything, so this never widens visibility. Used by + * getMyCommentedTicketsForRange for Progress "Supported". + */ + public function getTicketIdsCommentedByUser(int $userId, string $fromDate, string $toDate): array + { + return $this->connection->table('zp_comment') + ->where('module', 'ticket') + ->where('userId', $userId) + ->whereBetween('date', [$fromDate.' 00:00:00', $toDate.' 23:59:59']) + // moduleId is nullable in the schema; skip NULLs at the query level so + // (int) NULL doesn't inject a bogus id 0 into the downstream whereIn(). + ->whereNotNull('moduleId') + ->distinct() + ->pluck('moduleId') + ->map(fn ($id) => (int) $id) + ->filter() // belt-and-suspenders: drop any 0 (e.g. an empty-string id) + ->values() + ->all(); + } + + /** + * Ticket rows for the given ids, constrained to the given projects (the + * caller's access boundary). One row per ticket with the fields mobile + * user-ticket consumers expect (id, headline, projectId, projectName, + * editorId, userId, status, dateToFinish). Returns raw rows — no resolved + * statusLabel/statusClass. Used by getMyCommentedTicketsForRange. + */ + public function getTicketsByIdsWithinProjects(array $ticketIds, array $projectIds): array + { + if (empty($ticketIds) || empty($projectIds)) { + return []; + } + + $rows = $this->connection->table('zp_tickets') + ->leftJoin('zp_projects', 'zp_tickets.projectId', '=', 'zp_projects.id') + ->select( + 'zp_tickets.id', + 'zp_tickets.headline', + 'zp_tickets.projectId', + 'zp_tickets.editorId', + 'zp_tickets.userId', + 'zp_tickets.status', + 'zp_tickets.dateToFinish', + 'zp_projects.name as projectName', + ) + ->whereIn('zp_tickets.id', $ticketIds) + ->whereIn('zp_tickets.projectId', $projectIds) + // "Supported" is a task surface — exclude milestones the same way the + // other mobile user-ticket queries do, so a comment on a milestone + // doesn't surface a milestone row here. + ->where('zp_tickets.type', '<>', 'milestone') + // Stable, deterministic order (DB default order is unspecified). + ->orderBy('zp_tickets.dateToFinish') + ->orderBy('zp_tickets.id') + ->get(); + + return array_map(fn ($row) => (array) $row, $rows->all()); + } + /** * Get all tasks (and optionally subtasks) that belong to a milestone /** diff --git a/app/Domain/Tickets/Services/Tickets.php b/app/Domain/Tickets/Services/Tickets.php index 41e2420ff..e2a4d584e 100644 --- a/app/Domain/Tickets/Services/Tickets.php +++ b/app/Domain/Tickets/Services/Tickets.php @@ -2510,6 +2510,89 @@ public function getMyClosedTicketsForRange(?int $userId = null, ?string $from = return array_values($closed); } + /** + * @api + * + * Tickets the user COMMENTED on within [$from, $to] that they do NOT own + * (they're not the ticket's editor) — i.e. work they supported by weighing + * in on someone else's arc. Powers the mobile Progress "Supported" section + * (presence counts as much as production). + * + * Access safety: commented ticket ids are constrained to the PROJECTS the + * user can access (getProjectsUserHasAccessTo) — NOT to tickets they edit + * or collaborate on. Commenting on a ticket rarely makes you its editor or + * a collaborator, so scoping by those (as an earlier version did via + * simpleTicketQuery) silently dropped most supported work — it collapsed to + * "commented AND collaborator" and could return empty even when comments + * exist. Project access is the correct, still-safe boundary: a historical + * comment can't surface a ticket in a project the user no longer sees. The + * ownership filter then drops tickets the user is the editor of — those are + * "your work," not support. Defaults either bound to today. + * + * A non-admin may only read their OWN commented tickets: a caller-supplied + * $userId for someone else is forced back to the session user (IDOR guard, + * matching Projects::getProjectsUserHasAccessTo()). + * + * @return array> Raw ticket rows (id, headline, + * projectName, editorId, status, …). + * + * Note: these rows do NOT carry the resolved statusLabel / statusClass / + * statusType that the getAll*UserTickets methods attach. + */ + #[RequiresPermission(TicketsPermissions::VIEW)] + public function getMyCommentedTicketsForRange(?int $userId = null, ?string $from = null, ?string $to = null): array + { + $sessionUser = (int) session('userdata.id'); + $userId = $userId ?: $sessionUser; + // IDOR guard: reading someone else's comment activity requires admin. + if ($userId !== $sessionUser && ! Auth::userIsAtLeast(Roles::$admin)) { + $userId = $sessionUser; + } + if ($userId === 0) { + return []; + } + // Resolve "today" once so a run across midnight can't disagree on bounds. + $today = date('Y-m-d'); + $from = $from ?: $today; + $to = $to ?: $today; + if ($from > $to) { + [$from, $to] = [$to, $from]; + } + + $commentedIds = $this->ticketRepository->getTicketIdsCommentedByUser($userId, $from, $to); + if (empty($commentedIds)) { + return []; + } + + // Scope to the projects the user can see — the correct access boundary + // for "tickets I commented on" (you comment on others' work without + // being its editor/collaborator, so filtering by those would drop it). + $projects = $this->projectService->getProjectsUserHasAccessTo($userId); + if (! is_array($projects) || empty($projects)) { + return []; + } + $projectIds = array_values(array_filter(array_map( + fn ($p) => (int) ($p['id'] ?? 0), + $projects + ))); + if (empty($projectIds)) { + return []; + } + + $tickets = $this->ticketRepository->getTicketsByIdsWithinProjects($commentedIds, $projectIds); + + // "Supported", not "yours": drop tickets the user is the editor of. + $out = []; + foreach ($tickets as $ticket) { + if ((string) ($ticket['editorId'] ?? '') === (string) $userId) { + continue; + } + $out[] = $ticket; + } + + return array_values($out); + } + /** * @api * diff --git a/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php b/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php index ff4912d9c..15e4ff0a9 100644 --- a/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php +++ b/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php @@ -637,4 +637,155 @@ public function test_closed_tickets_range_forces_session_user_for_non_admin(): v $this->assertSame(1, $capturedUserId, 'a non-admin must not read another user\'s closures — userId is forced to the session user'); } + + /** + * Builds a service with a specific ticket repository AND project service — + * the two deps getMyCommentedTicketsForRange exercises. + */ + private function buildServiceWithTicketRepoAndProjectService( + TicketRepository $ticketRepository, + ProjectService $projectService + ): TicketsService { + return new TicketsService( + language: $this->make(LanguageCore::class), + ticketRepository: $ticketRepository, + timesheetsRepo: $this->make(TimesheetRepository::class), + settingsRepo: $this->make(SettingRepository::class), + projectService: $projectService, + timesheetService: $this->make(TimesheetService::class), + sprintService: $this->make(SprintService::class), + ticketHistoryRepo: $this->make(TicketHistory::class), + goalcanvasService: $this->make(Goalcanvas::class), + dateTimeHelper: $this->make(DateTimeHelper::class), + commentService: $this->make(CommentService::class), + clientService: $this->make(ClientService::class) + ); + } + + /** + * Supported = tickets you commented on within accessible projects, minus + * the ones you're the editor of. Editor-owned tickets are dropped; tickets + * outside the project-scoped fetch never appear. + */ + public function test_commented_tickets_range_excludes_owned_and_scopes_by_projects(): void + { + session(['userdata' => ['id' => 1]]); + + $ticketRepository = $this->make(TicketRepository::class, [ + 'getTicketIdsCommentedByUser' => fn (...$args) => [10, 20, 30], + // Project-scoped fetch only returns 10 + 20 (30 is outside access). + 'getTicketsByIdsWithinProjects' => fn (...$args) => [ + ['id' => 10, 'headline' => 'A', 'editorId' => '99', 'projectId' => 5, 'projectName' => 'P'], + ['id' => 20, 'headline' => 'B', 'editorId' => '1', 'projectId' => 5, 'projectName' => 'P'], + ], + ]); + $projectService = $this->make(ProjectService::class, [ + 'getProjectsUserHasAccessTo' => fn (...$args) => [['id' => 5], ['id' => 7]], + ]); + + $service = $this->buildServiceWithTicketRepoAndProjectService($ticketRepository, $projectService); + + $result = $service->getMyCommentedTicketsForRange(1, '2026-07-01', '2026-07-07'); + + // 20 is the user's own (editorId === 1) → excluded; 30 wasn't returned + // by the project-scoped fetch → absent. Only 10 remains. + $this->assertCount(1, $result); + $this->assertEquals(10, $result[0]['id']); + } + + /** + * No accessible projects → empty, without ever fetching tickets. + */ + public function test_commented_tickets_range_empty_without_project_access(): void + { + session(['userdata' => ['id' => 1]]); + + $ticketRepository = $this->make(TicketRepository::class, [ + 'getTicketIdsCommentedByUser' => fn (...$args) => [10], + 'getTicketsByIdsWithinProjects' => fn (...$args) => [['id' => 10, 'editorId' => '99']], + ]); + $projectService = $this->make(ProjectService::class, [ + 'getProjectsUserHasAccessTo' => fn (...$args) => false, + ]); + + $service = $this->buildServiceWithTicketRepoAndProjectService($ticketRepository, $projectService); + + $this->assertSame([], $service->getMyCommentedTicketsForRange(1, '2026-07-01', '2026-07-07')); + } + + public function test_commented_tickets_range_forces_session_user_for_non_admin(): void + { + session(['userdata' => ['id' => 1]]); + $capturedUserId = 'unset'; + + $ticketRepository = $this->make(TicketRepository::class, [ + 'getTicketIdsCommentedByUser' => function (...$args) use (&$capturedUserId) { + $capturedUserId = $args[0] ?? null; + + return []; + }, + ]); + $projectService = $this->make(ProjectService::class, [ + 'getProjectsUserHasAccessTo' => fn (...$args) => [['id' => 5]], + ]); + + $service = $this->buildServiceWithTicketRepoAndProjectService($ticketRepository, $projectService); + + // Non-admin supplies someone else's id — forced back to the session user. + $service->getMyCommentedTicketsForRange(999, '2026-07-01', '2026-07-07'); + + $this->assertSame(1, $capturedUserId, 'a non-admin must not read another user\'s comment activity — userId forced to session user'); + } + + public function test_commented_tickets_range_normalizes_reversed_range(): void + { + session(['userdata' => ['id' => 1]]); + $capturedFrom = null; + $capturedTo = null; + + $ticketRepository = $this->make(TicketRepository::class, [ + 'getTicketIdsCommentedByUser' => function (...$args) use (&$capturedFrom, &$capturedTo) { + $capturedFrom = $args[1] ?? null; + $capturedTo = $args[2] ?? null; + + return []; + }, + ]); + $projectService = $this->make(ProjectService::class, [ + 'getProjectsUserHasAccessTo' => fn (...$args) => [['id' => 5]], + ]); + + $service = $this->buildServiceWithTicketRepoAndProjectService($ticketRepository, $projectService); + + // Reversed on purpose — must be swapped earliest-first before the query. + $service->getMyCommentedTicketsForRange(1, '2026-07-12', '2026-07-05'); + + $this->assertSame('2026-07-05', $capturedFrom, 'range normalized earliest-first'); + $this->assertSame('2026-07-12', $capturedTo); + } + + public function test_commented_tickets_range_short_circuits_when_no_comments(): void + { + session(['userdata' => ['id' => 1]]); + $fetchCalled = false; + + $ticketRepository = $this->make(TicketRepository::class, [ + 'getTicketIdsCommentedByUser' => fn (...$args) => [], // nothing commented + 'getTicketsByIdsWithinProjects' => function (...$args) use (&$fetchCalled) { + $fetchCalled = true; + + return []; + }, + ]); + $projectService = $this->make(ProjectService::class, [ + 'getProjectsUserHasAccessTo' => fn (...$args) => [['id' => 5]], + ]); + + $service = $this->buildServiceWithTicketRepoAndProjectService($ticketRepository, $projectService); + + $result = $service->getMyCommentedTicketsForRange(1, '2026-07-01', '2026-07-07'); + + $this->assertSame([], $result); + $this->assertFalse($fetchCalled, 'an empty commented set must short-circuit before the ticket fetch'); + } }