Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions app/Domain/Tickets/Services/Tickets.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Comment on lines +2421 to 2423
#[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];
}
Comment on lines +2456 to +2475
Comment on lines +2468 to +2475

// Candidates: the user's currently-DONE tickets (statusType resolved).
$doneTickets = $this->getAllDoneUserTickets($userId);
if (empty($doneTickets)) {
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
71 changes: 71 additions & 0 deletions tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +553 to +557

$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');
}
}
Loading