diff --git a/app/Domain/Tickets/Services/Tickets.php b/app/Domain/Tickets/Services/Tickets.php index b8a7362620..0d7712f08f 100644 --- a/app/Domain/Tickets/Services/Tickets.php +++ b/app/Domain/Tickets/Services/Tickets.php @@ -2418,12 +2418,62 @@ public function getAllDoneUserTickets(?int $userId = null, ?int $project = null) * then re-completed the same day is included once. Built on the general * getStatusChangeEvents primitive, so throughput/burndown and strategy * reporting can reuse the same query. + * + * Thin wrapper over {@see getMyClosedTicketsForRange} with from == to. */ #[RequiresPermission(TicketsPermissions::VIEW)] public function getMyClosedTicketsForDate(?int $userId = null, ?string $date = null): array { $date = $date ?: date('Y-m-d'); + return $this->getMyClosedTicketsForRange($userId, $date, $date); + } + + /** + * @api + * + * Range form of {@see getMyClosedTicketsForDate}: the user's tasks marked + * DONE anywhere within [$from, $to] (inclusive, dates 'Y-m-d'), each + * annotated with `dateClosed` (the completion timestamp). Powers the mobile + * Progress "Closed this week / this month" sections — completed arcs are + * keyed on close-date within the period, independent of how recently the + * project was otherwise touched, so finished work never disappears. + * + * Same "closed" definition as the single-date form: the ticket is currently + * DONE and its status was changed to that DONE status within the range. A + * ticket completed more than once in the range is included once, keyed to + * its latest completion (events are newest-first). + * + * Bounds: an omitted $from or $to defaults to today; the range is then + * normalized so the earlier date is the lower bound (a reversed range is + * swapped rather than returning nothing). So passing only one bound yields + * the span between that date and today, in date order. + * + * A non-admin may only read their OWN closures: a caller-supplied $userId + * for someone else is forced back to the session user (IDOR guard). + */ + #[RequiresPermission(TicketsPermissions::VIEW)] + public function getMyClosedTicketsForRange(?int $userId = null, ?string $from = null, ?string $to = null): array + { + $sessionUser = (int) session('userdata.id'); + $userId = $userId ?: $sessionUser; + // IDOR guard: reading another user's closures 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; + // Tolerate a reversed range rather than returning nothing. + if ($from > $to) { + [$from, $to] = [$to, $from]; + } + // Candidates: the user's currently-DONE tickets (statusType resolved). $doneTickets = $this->getAllDoneUserTickets($userId); if (empty($doneTickets)) { @@ -2435,7 +2485,7 @@ public function getMyClosedTicketsForDate(?int $userId = null, ?string $date = n $byId[$ticket['id']] = $ticket; } - $events = $this->ticketRepository->getStatusChangeEvents(array_keys($byId), $date, $date); + $events = $this->ticketRepository->getStatusChangeEvents(array_keys($byId), $from, $to); $closed = []; foreach ($events as $event) { @@ -2446,8 +2496,8 @@ public function getMyClosedTicketsForDate(?int $userId = null, ?string $date = n } // Only count changes INTO the ticket's current (DONE) status — it - // was actually marked done that day, not merely touched. Events are - // newest-first, so the first match keeps the latest completion time. + // was actually marked done in the range, not merely touched. Events + // are newest-first, so the first match keeps the latest completion. if ((string) $event['changeValue'] === (string) $ticket['status']) { $ticket['dateClosed'] = $event['dateModified']; $closed[$ticketId] = $ticket; diff --git a/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php b/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php index 53b5e73628..91a49647fa 100644 --- a/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php +++ b/tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php @@ -544,4 +544,75 @@ public function test_get_all_milestones_unscoped_returns_empty_and_skips_reposit $this->assertSame([], $result); $this->assertFalse($called, 'repository should not be queried when criteria are not project-scoped'); } + + /** + * getMyClosedTicketsForRange: a reversed range is normalized (earlier date + * first), only status changes INTO the ticket's current DONE status count, + * and a ticket completed more than once keeps its latest completion. + */ + public function test_closed_tickets_range_normalizes_swapped_range_and_keeps_latest_completion(): void + { + session(['userdata' => ['id' => 1]]); + $capturedFrom = null; + $capturedTo = null; + + $ticketRepository = $this->make(TicketRepository::class, [ + 'simpleTicketQuery' => fn (...$args) => [ + ['id' => 10, 'type' => 'task', 'projectId' => 5, 'status' => 0], + ['id' => 20, 'type' => 'task', 'projectId' => 5, 'status' => 0], + ], + 'getStateLabels' => fn (...$args) => [ + 0 => ['statusType' => 'DONE', 'name' => 'Done', 'class' => ''], + 3 => ['statusType' => 'INPROGRESS', 'name' => 'In Progress', 'class' => ''], + ], + 'getStatusChangeEvents' => function ($ids, $from, $to) use (&$capturedFrom, &$capturedTo) { + $capturedFrom = $from; + $capturedTo = $to; + + return [ + ['ticketId' => 10, 'changeValue' => 0, 'dateModified' => '2026-07-10 10:00:00'], + ['ticketId' => 10, 'changeValue' => 0, 'dateModified' => '2026-07-09 09:00:00'], + ['ticketId' => 20, 'changeValue' => 3, 'dateModified' => '2026-07-10 10:00:00'], + ]; + }, + ]); + + $service = $this->buildServiceWithTicketRepository($ticketRepository); + + // Reversed range on purpose. + $result = $service->getMyClosedTicketsForRange(1, '2026-07-12', '2026-07-05'); + + $this->assertEquals('2026-07-05', $capturedFrom, 'range should be normalized earliest-first'); + $this->assertEquals('2026-07-12', $capturedTo); + + // 20's only event was a change to a non-DONE status → excluded. 10 kept + // to its latest completion (newest event wins). + $this->assertCount(1, $result); + $this->assertEquals(10, $result[0]['id']); + $this->assertEquals('2026-07-10 10:00:00', $result[0]['dateClosed']); + } + + public function test_closed_tickets_range_forces_session_user_for_non_admin(): void + { + // Non-admin session user (no admin role granted). + session(['userdata' => ['id' => 1]]); + $capturedUserId = 'unset'; + + $ticketRepository = $this->make(TicketRepository::class, [ + 'simpleTicketQuery' => function (...$args) use (&$capturedUserId) { + $capturedUserId = $args[0] ?? null; + + return []; // no done tickets — the asserted-on value is the userId + }, + 'getStateLabels' => fn (...$args) => [], + ]); + + $service = $this->buildServiceWithTicketRepository($ticketRepository); + + // Caller supplies SOMEONE ELSE's id — the IDOR guard must force it back + // to the session user before any query runs. + $service->getMyClosedTicketsForRange(999, '2026-07-01', '2026-07-10'); + + $this->assertSame(1, $capturedUserId, 'a non-admin must not read another user\'s closures — userId is forced to the session user'); + } }