Assign/archive/unarchive track no actor at all (for anyone, human or service account) #35

Closed
opened 2026-10-07 19:09:49 +00:00 by niklas · 2 comments
Owner

Summary

Follow-up to #25. While fixing #25 (service-account callers 500ing on incident-mutation
routes), it became clear three handlers in internal/api/incidents.go track no actor at
all today, for a human either
, let alone a service account:

  • handleIncidentAssign — writes assigned_to (who the incident is assigned to) and logs
    an evAssigned timeline event keyed on req.UserID, the assignee. The actor — who
    performed the assignment — is nowhere. The existing code comment says as much: "On an
    'assigned' event user_id is the assignee, not the actor."
  • handleIncidentArchive / handleIncidentUnarchive — flip archived_at and call no
    logEvent at all. There is no timeline entry and no actor for either operation, for anyone.

None of these 500 for a service account (there's no FK write to violate), so they were
correctly left out of #25's scope — but they're a real, pre-existing symmetry gap: every
other incident-mutation route (acknowledge, resolve, snooze, note) now records who acted,
distinctly for a human vs. a service account, and these three don't.

Suggested fix (not prescriptive)

  • handleIncidentArchive/handleIncidentUnarchive: add logEvent(..., evArchived/evUnarchived, userID, saID, nil, nil) calls using the callerActorIDs(ctx) helper #25 added in
    internal/api/incident_store.go. Needs new evArchived/evUnarchived event-type constants
    (internal/api/incident_store.go's const block) and corresponding web UI
    (internal/web/static/js/incident.js's eventText) / TUI timeline rendering for them.
  • handleIncidentAssign: recording the actor needs a schema decision analogous to #25's —
    either a new assigned_by/assigned_by_service_account_id pair of columns, or folding actor
    attribution into the existing evAssigned event's detail rather than its user_id (which
    must stay the assignee). If a column is added, this is also the natural point to add
    assigned_to_service_account_id (deliberately deferred in #25's migration 015, since nothing
    populated it yet) — i.e. service accounts should become assignable too, not just actors.

Also deferred from #25

  • terdut-tui rendering: #25 added acknowledged_by_service_account_id/
    acknowledged_by_service_account (on Incident) and service_account_id/
    service_account_name (on IncidentEvent) to the server's JSON. They're additive/omitempty,
    so the TUI's existing parsing in internal/api/types.go is unaffected either way — but the
    TUI has no code to display them, so a human TUI user viewing an incident a service account
    acted on sees no actor at all today (same degraded-but-not-wrong behavior the web UI had
    before #25). Worth doing once this issue's columns settle, so both gaps get TUI parity in one
    pass rather than two.

References

  • internal/api/incidents.go — handleIncidentAssign (~line 285), handleIncidentArchive/
    handleIncidentUnarchive (~line 400).
  • internal/api/incident_store.go — callerActorIDs, logEvent, the ev* constants (added/
    extended by #25).
  • #25 — the fix this follows up on, and migration 015_incident_service_account_actors.sql.
## Summary Follow-up to #25. While fixing #25 (service-account callers 500ing on incident-mutation routes), it became clear three handlers in `internal/api/incidents.go` track **no actor at all today, for a human either**, let alone a service account: - `handleIncidentAssign` — writes `assigned_to` (who the incident is assigned *to*) and logs an `evAssigned` timeline event keyed on `req.UserID`, the assignee. The *actor* — who performed the assignment — is nowhere. The existing code comment says as much: "On an 'assigned' event user_id is the assignee, not the actor." - `handleIncidentArchive` / `handleIncidentUnarchive` — flip `archived_at` and call no `logEvent` at all. There is no timeline entry and no actor for either operation, for anyone. None of these 500 for a service account (there's no FK write to violate), so they were correctly left out of #25's scope — but they're a real, pre-existing symmetry gap: every *other* incident-mutation route (acknowledge, resolve, snooze, note) now records who acted, distinctly for a human vs. a service account, and these three don't. ## Suggested fix (not prescriptive) - `handleIncidentArchive`/`handleIncidentUnarchive`: add `logEvent(..., evArchived/evUnarchived, userID, saID, nil, nil)` calls using the `callerActorIDs(ctx)` helper #25 added in `internal/api/incident_store.go`. Needs new `evArchived`/`evUnarchived` event-type constants (`internal/api/incident_store.go`'s `const` block) and corresponding web UI (`internal/web/static/js/incident.js`'s `eventText`) / TUI timeline rendering for them. - `handleIncidentAssign`: recording the actor needs a schema decision analogous to #25's — either a new `assigned_by`/`assigned_by_service_account_id` pair of columns, or folding actor attribution into the existing `evAssigned` event's `detail` rather than its `user_id` (which must stay the assignee). If a column is added, this is also the natural point to add `assigned_to_service_account_id` (deliberately deferred in #25's migration 015, since nothing populated it yet) — i.e. service accounts should become assignable too, not just actors. ## Also deferred from #25 - **`terdut-tui` rendering**: #25 added `acknowledged_by_service_account_id`/ `acknowledged_by_service_account` (on `Incident`) and `service_account_id`/ `service_account_name` (on `IncidentEvent`) to the server's JSON. They're additive/omitempty, so the TUI's existing parsing in `internal/api/types.go` is unaffected either way — but the TUI has no code to *display* them, so a human TUI user viewing an incident a service account acted on sees no actor at all today (same degraded-but-not-wrong behavior the web UI had before #25). Worth doing once this issue's columns settle, so both gaps get TUI parity in one pass rather than two. ## References - `internal/api/incidents.go` — `handleIncidentAssign` (~line 285), `handleIncidentArchive`/ `handleIncidentUnarchive` (~line 400). - `internal/api/incident_store.go` — `callerActorIDs`, `logEvent`, the `ev*` constants (added/ extended by #25). - #25 — the fix this follows up on, and migration `015_incident_service_account_actors.sql`.
Author
Owner

Implemented on main (not yet pushed or released).

Done

  • Migration 018_incident_event_actor.sql: incident_events.actor_user_id / actor_service_account_id (exclusive, ON DELETE SET NULL). Used only for assigned events, where user_id stays the assignee. Older assignments have no actor.
  • handleIncidentAssign records the caller as actor (human or service account).
  • Archive/unarchive now log new archived / unarchived events with the caller.
  • Timeline JSON adds actor_user_id, actor_username, actor_service_account_id, actor_service_account_name (omitempty); web timeline shows "Assigned to X by Y" and archive/unarchive entries; README updated.
  • Tests cover human and service-account callers for all three routes. make fmt lint test helm-lint is green.

Deferred

  • Service accounts are still not assignable (assigned_to_service_account_id): that changes the assign request body, the assignee picker, the notifier and the assigned_to filter, so it should be its own issue.
  • terdut-tui rendering of the #25 fields (service_account*) and the new actor_* fields and archived/unarchived events. The additions are additive, so the TUI keeps working meanwhile.
Implemented on `main` (not yet pushed or released). **Done** - Migration `018_incident_event_actor.sql`: `incident_events.actor_user_id` / `actor_service_account_id` (exclusive, `ON DELETE SET NULL`). Used only for `assigned` events, where `user_id` stays the assignee. Older assignments have no actor. - `handleIncidentAssign` records the caller as actor (human or service account). - Archive/unarchive now log new `archived` / `unarchived` events with the caller. - Timeline JSON adds `actor_user_id`, `actor_username`, `actor_service_account_id`, `actor_service_account_name` (omitempty); web timeline shows "Assigned to X by Y" and archive/unarchive entries; README updated. - Tests cover human and service-account callers for all three routes. `make fmt lint test helm-lint` is green. **Deferred** - Service accounts are still not assignable (`assigned_to_service_account_id`): that changes the assign request body, the assignee picker, the notifier and the `assigned_to` filter, so it should be its own issue. - `terdut-tui` rendering of the #25 fields (`service_account*`) and the new `actor_*` fields and `archived`/`unarchived` events. The additions are additive, so the TUI keeps working meanwhile.
Author
Owner

Shipped in v0.39.0 (image published, trivy scan clean). Deploy is pending the wrapper-chart PR: Ryuvia/charts#299. Closing. The two deferred items (assignable service accounts, and terdut-tui rendering of the actor fields) still need their own issues.

Shipped in v0.39.0 (image published, trivy scan clean). Deploy is pending the wrapper-chart PR: https://git.ryuvia.com/Ryuvia/charts/pulls/299. Closing. The two deferred items (assignable service accounts, and `terdut-tui` rendering of the actor fields) still need their own issues.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: niklas/terdut-server#35