Repository navigation
feat: implement ticket visibility controls (#44) - #57
Conversation
| async listRecent(limit: number): Promise<TicketRecord[]> { | ||
| async listRecent(limit: number, viewer: TicketViewer | null = null): Promise<TicketRecord[]> { | ||
| const visibilityFilter = | ||
| viewer?.role === "TECHNICIAN" |
There was a problem hiding this comment.
Test gap (please add): none of the tests exercise this where filter. The route tests use a fake repository whose listRecent ignores the viewer, and the route filters the list again with canAccessTicket. If this SQL filter broke, every test would still pass, but hidden tickets would use up the 50-ticket list limit and visible tickets would quietly drop off the list. Could you add a case to postgres-ticket-repository.integration.test.ts? Create one ticket of each visibility, then check what listRecent returns for null, the owner, another user and a technician.
Simplification (nit): the technician case matches all three values, which is the same as no filter. viewer?.role === "TECHNICIAN" ? undefined : … says that more directly, and Drizzle's .where(undefined) is a no-op.
| additionalInformation: ticket.additionalInformation, | ||
| status: ticket.status, | ||
| visibility: ticket.visibility, | ||
| canReply: canAccessTicket(ticket, viewer), |
There was a problem hiding this comment.
canReply uses the same check as viewing, so it's true for every ticket a user can see. That makes the "you can view but cannot reply" message in TicketReplies.tsx unreachable. If view and reply permissions are expected to differ later, a short comment saying so would help. Otherwise we could drop the field and the dead branch for now.
| </form> | ||
| </form> | ||
| ) : ( | ||
| <p className="field-hint">You can view this conversation, but you cannot reply.</p> |
There was a problem hiding this comment.
Related to the canReply comment: this branch can't be reached at the moment.
| return { | ||
| PUBLIC: "Public", | ||
| AUTHENTICATED: "Logged-in users", | ||
| PRIVATE: "You and IT only" |
There was a problem hiding this comment.
Nit: technicians also see this badge on other people's tickets, where "You and IT only" reads oddly. Maybe just "Private"? The form's dropdown can keep its longer explanation.
| } from "./ticket-contract.js"; | ||
| import type { ReplyRecord, TicketRecord, TicketRepository } from "./ticket-repository.js"; | ||
| import { canAccessTicket } from "./ticket-access.js"; | ||
| import type { TicketViewer } from "./ticket-repository.js"; |
There was a problem hiding this comment.
Nit: this can be folded into the existing import type { … } from "./ticket-repository.js" above (same in postgres-ticket-repository.ts line 13).
| }; | ||
| } | ||
|
|
||
| async function currentUser( |
There was a problem hiding this comment.
Nit: the assignment route (PATCH /api/tickets/:ticketId/assignment) still looks up the session inline instead of using this helper. After merging main, that route should use #56's requireTechnician guard anyway, so it may sort itself out during the conflict fix.
Preserve ticket visibility controls while incorporating PR #56 IT account support, shared technician authorization, PostgreSQL visibility filtering tests, and ADR numbering updates.
Preserve ticket visibility and IT authorization while incorporating PR #58 technician status transitions, API contracts, UI controls, and documentation.
Resolve conflicts with ticket visibility (#57): - Renumber the notifications migration to 0006 after main's 0005_ticket_visibility; the regenerated SQL is unchanged. - Keep both canTechnicianChangeStatus and the visibility types in ticket-contract.ts, and pass notificationRepository to main's new route tests. - Assert the status PATCH with toHaveBeenCalledWith in TicketDetail: the replies panel can fetch after the click, which made the last-call assertion flaky in CI.
Summary
Implements ticket visibility options, allowing users to control who can view and reply to their support tickets.
Closes #44
Changes
Testing
Notes