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
63 changes: 63 additions & 0 deletions app/Domain/Tickets/Repositories/Tickets.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment on lines +1960 to +1963

return array_map(fn ($row) => (array) $row, $rows->all());
}

/**
* Get all tasks (and optionally subtasks) that belong to a milestone
/**
Expand Down
83 changes: 83 additions & 0 deletions app/Domain/Tickets/Services/Tickets.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +2521 to +2526
* 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<int, array<string, mixed>> 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
{
Comment on lines +2542 to +2544
$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;
Comment on lines +2554 to +2557
if ($from > $to) {
[$from, $to] = [$to, $from];
}
Comment on lines +2542 to +2560

$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);

Comment on lines +2567 to +2583
// "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
*
Expand Down
151 changes: 151 additions & 0 deletions tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
Comment on lines +645 to +663

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