diff --git a/README.md b/README.md index dd6b7b7..16d03d6 100644 --- a/README.md +++ b/README.md @@ -984,9 +984,11 @@ name: degrade unknown values to "resolved, reason unknown". | `created_at` | timestamp | | Types written today: `triggered`, `alert_added`, `alert_resolved`, -`acknowledged`, `unacknowledged`, `assigned`, `snoozed`, `unsnoozed`, `resolved`, -`note`, `notified`, `notify_failed`, `deadman_silent`. On an `assigned` event -`user_id` is the **assignee**, not the actor. New types may be added; render +`acknowledged`, `unacknowledged`, `assigned`, `archived`, `unarchived`, `snoozed`, +`unsnoozed`, `resolved`, `note`, `notified`, `notify_failed`, `deadman_silent`. On an +`assigned` event `user_id` is the **assignee**, not the actor; the actor is in +`actor_user_id`/`actor_username` or `actor_service_account_id`/`actor_service_account_name` +(absent on assignments made before they were recorded). New types may be added; render unknown ones generically rather than dropping them. On `notified` and `notify_failed`, `detail` carries the notification kind diff --git a/internal/api/incident_store.go b/internal/api/incident_store.go index 6fede5d..cd7aafb 100644 --- a/internal/api/incident_store.go +++ b/internal/api/incident_store.go @@ -32,6 +32,8 @@ const ( evAcknowledged = "acknowledged" evUnacknowledged = "unacknowledged" evAssigned = "assigned" + evArchived = "archived" + evUnarchived = "unarchived" evSnoozed = "snoozed" evUnsnoozed = "unsnoozed" evResolved = "resolved" @@ -143,6 +145,17 @@ func logEvent(ctx context.Context, q querier, incidentID int64, evType string, u return err } +// logAssignedEvent records an assignment: user_id is the assignee, and the +// caller who performed it goes in the actor_* columns (migration 018), since +// user_id cannot hold both. +func logAssignedEvent(ctx context.Context, q querier, incidentID, assigneeID int64, actorUserID, actorServiceAccountID *int64) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO incident_events (incident_id, type, user_id, actor_user_id, actor_service_account_id, created_at) + VALUES ($1, $2, $3, $4, $5, $6)`, + incidentID, evAssigned, assigneeID, actorUserID, actorServiceAccountID, time.Now().Unix()) + return err +} + // todayUTC is the schedule's day key. The schedule's smallest unit is one UTC day. func todayUTC() string { return time.Now().UTC().Format("2006-01-02") diff --git a/internal/api/incidents.go b/internal/api/incidents.go index 8714fdd..b7b6c5e 100644 --- a/internal/api/incidents.go +++ b/internal/api/incidents.go @@ -158,10 +158,14 @@ func handleIncidentTimeline(db *sql.DB) http.HandlerFunc { rows, err := db.QueryContext(r.Context(), ` SELECT e.id, e.incident_id, e.type, e.user_id, u.username, e.service_account_id, sa.name, + e.actor_user_id, au.username, + e.actor_service_account_id, asa.name, e.alert_id, e.detail, e.created_at FROM incident_events e LEFT JOIN users u ON u.id = e.user_id LEFT JOIN service_accounts sa ON sa.id = e.service_account_id + LEFT JOIN users au ON au.id = e.actor_user_id + LEFT JOIN service_accounts asa ON asa.id = e.actor_service_account_id WHERE e.incident_id = $1 ORDER BY e.created_at ASC, e.id ASC`, id) if err != nil { @@ -176,6 +180,8 @@ func handleIncidentTimeline(db *sql.DB) http.HandlerFunc { var ts int64 if err := rows.Scan(&e.ID, &e.IncidentID, &e.Type, &e.UserID, &e.Username, &e.ServiceAccountID, &e.ServiceAccountName, + &e.ActorUserID, &e.ActorUsername, + &e.ActorServiceAccountID, &e.ActorServiceAccountName, &e.AlertID, &e.Detail, &ts); err != nil { respond(w, http.StatusInternalServerError, errResp("internal error")) return @@ -315,8 +321,10 @@ func handleIncidentAssign(db *sql.DB) http.HandlerFunc { req.UserID, id) { return } - // On an "assigned" event user_id is the assignee, not the actor. - if err := logEvent(r.Context(), db, id, evAssigned, &req.UserID, nil, nil, nil); err != nil { + // On an "assigned" event user_id is the assignee; the actor goes in + // the actor_* columns. + actorUserID, actorSAID := callerActorIDs(r.Context()) + if err := logAssignedEvent(r.Context(), db, id, req.UserID, actorUserID, actorSAID); err != nil { respond(w, http.StatusInternalServerError, errResp("internal error")) return } @@ -417,6 +425,11 @@ func handleIncidentArchive(db *sql.DB) http.HandlerFunc { respond(w, http.StatusNotFound, errResp("incident not found")) return } + userID, saID := callerActorIDs(r.Context()) + if err := logEvent(r.Context(), db, id, evArchived, userID, saID, nil, nil); err != nil { + respond(w, http.StatusInternalServerError, errResp("internal error")) + return + } respondIncident(w, r, db, id) } } @@ -437,6 +450,11 @@ func handleIncidentUnarchive(db *sql.DB) http.HandlerFunc { respond(w, http.StatusNotFound, errResp("incident not found")) return } + userID, saID := callerActorIDs(r.Context()) + if err := logEvent(r.Context(), db, id, evUnarchived, userID, saID, nil, nil); err != nil { + respond(w, http.StatusInternalServerError, errResp("internal error")) + return + } w.WriteHeader(http.StatusNoContent) } } diff --git a/internal/api/incidents_test.go b/internal/api/incidents_test.go index 98c0192..2fdca38 100644 --- a/internal/api/incidents_test.go +++ b/internal/api/incidents_test.go @@ -870,3 +870,105 @@ func contains(haystack []string, needle string) bool { } return false } + +// --------------------------------------------------------------------------- +// Actor on assign / archive / unarchive (terdut-server#35) +// --------------------------------------------------------------------------- + +// lastEvent returns the newest timeline event of the given type. +func lastEvent(t *testing.T, events []map[string]any, typ string) map[string]any { + t.Helper() + for i := len(events) - 1; i >= 0; i-- { + if events[i]["type"] == typ { + return events[i] + } + } + t.Fatalf("no %q event in %v", typ, eventTypes(events)) + return nil +} + +func TestIncident_AssignRecordsHumanActor(t *testing.T) { + s := newTS(t) + postWebhook(t, s, []map[string]any{ + amAlert("fp-asg-actor", "Assignable", "firing", "2026-05-20T10:00:00Z", zeroTime, nil), + }) + s.req(t, http.MethodPost, "/api/users", + map[string]string{"username": "alice", "email": "alice@test.com"}).Body.Close() + s.req(t, http.MethodPost, "/api/incidents/1/assign", map[string]any{"user_id": 2}).Body.Close() + + ev := lastEvent(t, timeline(t, s, 1), "assigned") + if ev["username"] != "alice" { + t.Errorf("expected the assignee alice in username, got %v", ev["username"]) + } + if ev["actor_user_id"] == nil || ev["actor_username"] == nil { + t.Errorf("expected the assigning human in actor_*, got %v", ev) + } + if ev["actor_service_account_id"] != nil { + t.Errorf("expected no service-account actor, got %v", ev["actor_service_account_id"]) + } +} + +func TestIncident_ArchiveUnarchiveRecordHumanActor(t *testing.T) { + s := newTS(t) + postWebhook(t, s, []map[string]any{ + amAlert("fp-arc-actor", "Archivable", "firing", "2026-05-20T10:00:00Z", zeroTime, nil), + }) + s.req(t, http.MethodPost, "/api/incidents/1/resolve", nil).Body.Close() + s.req(t, http.MethodPost, "/api/incidents/1/archive", nil).Body.Close() + s.req(t, http.MethodDelete, "/api/incidents/1/archive", nil).Body.Close() + + events := timeline(t, s, 1) + for _, typ := range []string{"archived", "unarchived"} { + ev := lastEvent(t, events, typ) + if ev["user_id"] == nil || ev["service_account_id"] != nil { + t.Errorf("%s: expected only the human actor, got %v", typ, ev) + } + } +} + +func TestServiceAccount_AssignArchiveUnarchiveRecordActor(t *testing.T) { + s := newTS(t) + instanceKey := createServiceAccount(t, s, s.key, "operator", models.ServiceAccountScopeInstance, 0) + teamA := createTeamAs(t, s, instanceKey, "team-a") + keyA := createServiceAccount(t, s, instanceKey, "team-a-sa", models.ServiceAccountScopeTeam, teamA) + var integration struct { + Key string `json:"key"` + } + decode(t, s.reqAs(t, keyA, http.MethodPost, "/api/teams/"+id64(teamA)+"/integrations", + map[string]string{"name": "test"}), &integration) + postToIntegration(t, s, integration.Key, "fp-sa-35", "SA35") // incident 1 + + resp := s.reqAs(t, keyA, http.MethodPost, "/api/incidents/1/assign", map[string]any{"user_id": 1}) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("service account assign: %d", resp.StatusCode) + } + resp = s.reqAs(t, keyA, http.MethodPost, "/api/incidents/1/resolve", nil) + resp.Body.Close() + resp = s.reqAs(t, keyA, http.MethodPost, "/api/incidents/1/archive", nil) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("service account archive: %d", resp.StatusCode) + } + resp = s.reqAs(t, keyA, http.MethodDelete, "/api/incidents/1/archive", nil) + resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("service account unarchive: %d", resp.StatusCode) + } + + var events []map[string]any + decode(t, s.reqAs(t, keyA, http.MethodGet, "/api/incidents/1/timeline", nil), &events) + asg := lastEvent(t, events, "assigned") + if asg["actor_service_account_id"] == nil || asg["actor_user_id"] != nil { + t.Errorf("assigned: expected only the service-account actor, got %v", asg) + } + if asg["user_id"] == nil { + t.Errorf("assigned: user_id must stay the assignee, got %v", asg) + } + for _, typ := range []string{"archived", "unarchived"} { + ev := lastEvent(t, events, typ) + if ev["service_account_id"] == nil || ev["user_id"] != nil { + t.Errorf("%s: expected only the service-account actor, got %v", typ, ev) + } + } +} diff --git a/internal/db/migrations/018_incident_event_actor.sql b/internal/db/migrations/018_incident_event_actor.sql new file mode 100644 index 0000000..cfee52f --- /dev/null +++ b/internal/db/migrations/018_incident_event_actor.sql @@ -0,0 +1,21 @@ +-- Who performed an assignment (terdut-server#35). On an 'assigned' event +-- incident_events.user_id is the assignee, so the actor needs columns of its +-- own. Only populated for 'assigned' events; every other event type keeps +-- using user_id/service_account_id for the actor. Older 'assigned' rows stay +-- NULL (the actor was never recorded). Same shape as migration 015: nullable, +-- mutually exclusive, ON DELETE SET NULL. +-- +-- assigned_to_service_account_id is still deliberately not added: making +-- service accounts assignable is a separate change (request body, assignee +-- picker, notifier, filters). +ALTER TABLE incident_events + ADD COLUMN actor_user_id BIGINT REFERENCES users(id) ON DELETE SET NULL, + ADD COLUMN actor_service_account_id BIGINT REFERENCES service_accounts(id) ON DELETE SET NULL; + +ALTER TABLE incident_events + ADD CONSTRAINT incident_events_assign_actor_xor_chk CHECK ( + actor_user_id IS NULL OR actor_service_account_id IS NULL + ); + +CREATE INDEX incident_events_actor_user_id_idx ON incident_events(actor_user_id); +CREATE INDEX incident_events_actor_service_account_id_idx ON incident_events(actor_service_account_id); diff --git a/internal/models/incident.go b/internal/models/incident.go index 7ce9bbd..c1847ca 100644 --- a/internal/models/incident.go +++ b/internal/models/incident.go @@ -74,7 +74,8 @@ type Incident struct { // and is the only history this server keeps — alert rows are mutated in place. // // Type is one of: triggered, alert_added, alert_resolved, acknowledged, -// unacknowledged, assigned, snoozed, unsnoozed, resolved, note. UserID and +// unacknowledged, assigned, archived, unarchived, snoozed, unsnoozed, +// resolved, note. UserID and // ServiceAccountID are mutually exclusive; both nil means the server acted // rather than any caller. type IncidentEvent struct { @@ -92,6 +93,14 @@ type IncidentEvent struct { ServiceAccountID *int64 `json:"service_account_id,omitempty"` ServiceAccountName *string `json:"service_account_name,omitempty"` + // Actor* name who performed an 'assigned' event, whose UserID is the + // assignee. Mutually exclusive; unset on every other event type and on + // assignments made before migration 018. See terdut-server#35. + ActorUserID *int64 `json:"actor_user_id,omitempty"` + ActorUsername *string `json:"actor_username,omitempty"` + ActorServiceAccountID *int64 `json:"actor_service_account_id,omitempty"` + ActorServiceAccountName *string `json:"actor_service_account_name,omitempty"` + AlertID *int64 `json:"alert_id,omitempty"` Detail *string `json:"detail,omitempty"` CreatedAt time.Time `json:"created_at"` diff --git a/internal/web/static/js/incident.js b/internal/web/static/js/incident.js index 856d3fc..4b69eda 100644 --- a/internal/web/static/js/incident.js +++ b/internal/web/static/js/incident.js @@ -225,6 +225,15 @@ function actorLabel(ev, named = false) { return null; } +// assignerLabel is actorLabel for an 'assigned' event, whose user_id is the +// assignee: the person who made the assignment is in the actor_* fields +// (null for assignments from before they were recorded). +function assignerLabel(ev, named = false) { + if (ev.actor_user_id != null) return named ? (ev.actor_username || 'someone') : who(ev.actor_user_id, ev.actor_username); + if (ev.actor_service_account_id != null) return ev.actor_service_account_name || 'a service account'; + return null; +} + // named spells users out instead of "you", for text that leaves this page. function eventText(ev, named = false) { const person = actorLabel(ev, named); @@ -239,7 +248,12 @@ function eventText(ev, named = false) { case 'alert_resolved': return [`Alert resolved: ${alertName()}`]; case 'acknowledged': return [strong(person), ' acknowledged']; case 'unacknowledged': return [strong(person), ' cleared the acknowledgement']; - case 'assigned': return ['Assigned to ', strong(person)]; + case 'assigned': { + const by = assignerLabel(ev, named); + return by ? ['Assigned to ', strong(person), ' by ', strong(by)] : ['Assigned to ', strong(person)]; + } + case 'archived': return [strong(person), ' archived the incident']; + case 'unarchived': return [strong(person), ' unarchived the incident']; case 'snoozed': return [strong(person), ` snoozed until ${ev.detail ? when(ev.detail) : '…'}`]; case 'unsnoozed': return [strong(person), ' ended the snooze']; case 'resolved': return person ? [strong(person), ' resolved the incident'] : ['Resolved: every alert stopped firing'];