From 814ef2c5e83ebbfda742b26d091335412d97394e Mon Sep 17 00:00:00 2001 From: Niklas Ye Date: Fri, 31 Jul 2026 07:24:09 +0200 Subject: [PATCH] fix: stop styled lines from indenting the text that follows An incident with nobody assigned and nobody holding it rendered its detail view like this: Assigned: nobody Acked: not acknowledged lipgloss pads every line of a styled block out to the width of its widest line. A trailing newline inside Render therefore produces a second line made entirely of padding, and the next write to the builder starts after that padding instead of at the left margin. Twelve call sites put the newline inside. Adding a line() helper that keeps the newline outside, and using it throughout the content builders. Shipped in v0.4.0 and only visible on the unassigned or unacknowledged path, which is why it survived the pre-release check: that run had somebody on call, so the incident was assigned and acknowledged and both lines took the styled-with-value branch instead. view_test.go covers the content builders, including the two states that were broken and an expired snooze not being reported as a snooze. --- internal/tui/view.go | 48 ++++--- internal/tui/view_test.go | 286 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 internal/tui/view_test.go diff --git a/internal/tui/view.go b/internal/tui/view.go index 67dc485..18bed57 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -365,6 +365,16 @@ func (m Model) renderStats() string { return m.statsViewport.View() } +// line renders s in a style and terminates it. +// +// The newline has to stay outside Render: lipgloss pads every line of a styled +// block out to its widest line, so a trailing newline inside the block produces +// a second line made entirely of padding, and whatever is written next starts +// after that padding instead of at the left margin. +func line(style lipgloss.Style, s string) string { + return style.Render(s) + "\n" +} + // ── Content builders ─────────────────────────────────────────────────────── func buildIncidentDetailContent(inc api.Incident, timeline []api.IncidentEvent, cursor, width int) string { @@ -391,7 +401,7 @@ func buildIncidentDetailContent(inc api.Incident, timeline []api.IncidentEvent, if inc.AssignedTo != "" { b.WriteString(fmt.Sprintf(" Assigned: %s\n", styleBold.Render(inc.AssignedTo))) } else { - b.WriteString(styleMuted.Render(" Assigned: nobody\n")) + b.WriteString(line(styleMuted, " Assigned: nobody")) } if inc.AcknowledgedByID != nil { @@ -399,14 +409,14 @@ func buildIncidentDetailContent(inc api.Incident, timeline []api.IncidentEvent, if inc.AcknowledgedAt != nil { ackAt = " at " + inc.AcknowledgedAt.UTC().Format("2006-01-02 15:04 UTC") } - b.WriteString(styleResolved.Render( - fmt.Sprintf(" Acked: %s%s\n", inc.AcknowledgedBy, ackAt))) + b.WriteString(line(styleResolved, + fmt.Sprintf(" Acked: %s%s", inc.AcknowledgedBy, ackAt))) } else { - b.WriteString(styleMuted.Render(" Acked: not acknowledged\n")) + b.WriteString(line(styleMuted, " Acked: not acknowledged")) } if inc.IsSnoozed() { - b.WriteString(styleSnoozed.Render(fmt.Sprintf(" Snoozed: until %s (%s)\n", + b.WriteString(line(styleSnoozed, fmt.Sprintf(" Snoozed: until %s (%s)", inc.SnoozedUntil.UTC().Format("2006-01-02 15:04 UTC"), humanUntil(now, *inc.SnoozedUntil)))) } @@ -419,8 +429,8 @@ func buildIncidentDetailContent(inc api.Incident, timeline []api.IncidentEvent, inc.ResolvedAt.UTC().Format("2006-01-02 15:04 UTC"), humanAgo(now, *inc.ResolvedAt), source)) } if inc.ArchivedAt != nil { - b.WriteString(styleMuted.Render(" Archived: " + - inc.ArchivedAt.UTC().Format("2006-01-02 15:04 UTC") + "\n")) + b.WriteString(line(styleMuted, " Archived: "+ + inc.ArchivedAt.UTC().Format("2006-01-02 15:04 UTC"))) } b.WriteString("\n") @@ -436,7 +446,7 @@ func buildIncidentDetailContent(inc api.Incident, timeline []api.IncidentEvent, // Member alerts b.WriteString(divider(fmt.Sprintf("Alerts (%d)", len(inc.Alerts)), width)) if len(inc.Alerts) == 0 { - b.WriteString(styleMuted.Render(" No alerts.\n")) + b.WriteString(line(styleMuted, " No alerts.")) } else { for _, a := range inc.Alerts { marker := styleFiring.Render("●") @@ -458,7 +468,7 @@ func buildIncidentDetailContent(inc api.Incident, timeline []api.IncidentEvent, notes := noteEvents(timeline) b.WriteString(divider(fmt.Sprintf("Timeline (%d events, %d notes)", len(timeline), len(notes)), width)) if len(timeline) == 0 { - b.WriteString(styleMuted.Render(" Nothing recorded yet.\n")) + b.WriteString(line(styleMuted, " Nothing recorded yet.")) } else { noteIndex := 0 for _, e := range timeline { @@ -584,7 +594,7 @@ func buildAlertDetailContent(alert api.Alert, width int) string { styleBold.Render(fmt.Sprintf("#%d", *alert.IncidentID)), styleMuted.Render("press i to open it"))) } else { - b.WriteString(styleMuted.Render(" Incident: none\n")) + b.WriteString(line(styleMuted, " Incident: none")) } b.WriteString("\n") @@ -606,8 +616,8 @@ func buildAlertDetailContent(alert api.Alert, width int) string { // Alerts carry no workflow state: it all lives on the incident. b.WriteString(divider("", width)) - b.WriteString(styleMuted.Render( - " Alerts are read-only — acknowledge, assign, note and resolve on the incident.\n")) + b.WriteString(line(styleMuted, + " Alerts are read-only — acknowledge, assign, note and resolve on the incident.")) return b.String() } @@ -627,7 +637,7 @@ func buildStatsContent(incidents *api.IncidentStats, top []api.TopAlert, byHour // Response times first: they are what a rota is actually judged on. b.WriteString(divider("Incident Response", width)) if incidents == nil { - b.WriteString(styleMuted.Render(" No data.\n")) + b.WriteString(line(styleMuted, " No data.")) } else { b.WriteString(fmt.Sprintf(" %-28s %s\n", "Incidents total", styleBold.Render(fmt.Sprintf("%d", incidents.Total)))) @@ -642,14 +652,14 @@ func buildStatsContent(incidents *api.IncidentStats, top []api.TopAlert, byHour b.WriteString(fmt.Sprintf(" %-28s %s\n", "Mean time to resolve", styleBold.Render(humanSeconds(incidents.MTTRSeconds)))) if incidents.MTTASeconds == nil || incidents.MTTRSeconds == nil { - b.WriteString(styleMuted.Render(" (— means nothing has been acknowledged or resolved yet)\n")) + b.WriteString(line(styleMuted, " (— means nothing has been acknowledged or resolved yet)")) } } b.WriteString("\n") b.WriteString(divider("Top Alerts", width)) if len(top) == 0 { - b.WriteString(styleMuted.Render(" No data.\n")) + b.WriteString(line(styleMuted, " No data.")) } else { maxCount := top[0].Count for i, a := range top { @@ -672,7 +682,7 @@ func buildStatsContent(incidents *api.IncidentStats, top []api.TopAlert, byHour b.WriteString(fmt.Sprintf(" %2dh %-*s %d\n", h.Hour, barWidth, bar, h.Count)) } } else { - b.WriteString(styleMuted.Render(" No data.\n")) + b.WriteString(line(styleMuted, " No data.")) } b.WriteString("\n") @@ -689,7 +699,7 @@ func buildStatsContent(incidents *api.IncidentStats, top []api.TopAlert, byHour b.WriteString(fmt.Sprintf(" %-4s %-*s %d\n", d.DayName[:3], barWidth, bar, d.Count)) } } else { - b.WriteString(styleMuted.Render(" No data.\n")) + b.WriteString(line(styleMuted, " No data.")) } return b.String() @@ -723,7 +733,7 @@ func (m Model) renderUserCreate() string { func (m Model) renderAPIKeyMenu() string { header := fmt.Sprintf("\n API keys for %s\n", styleBold.Render(m.selectedUser.Username)) - warning := styleMuted.Render(" Keys cannot be listed — only new keys can be created,\n or existing ones revoked by their integer ID.\n") + warning := line(styleMuted, " Keys cannot be listed — only new keys can be created,\n or existing ones revoked by their integer ID.") options := "\n" + styleAccent.Render(" n") + " · create a new API key\n" + styleAccent.Render(" r") + " · revoke a key by ID\n" @@ -757,7 +767,7 @@ func (m Model) renderAPIKeyReveal() string { func (m Model) renderAPIKeyRevokeByID() string { header := fmt.Sprintf("\n Revoke API key for %s\n", styleBold.Render(m.selectedUser.Username)) - hint := styleMuted.Render(" Enter the integer key ID (shown when the key was created).\n") + hint := line(styleMuted, " Enter the integer key ID (shown when the key was created).") label := styleSelected.Render(" Key ID: ") return header + "\n" + hint + "\n" + label + m.apiKeyRevokeInput.View() + "\n" } diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go new file mode 100644 index 0000000..e48fd0b --- /dev/null +++ b/internal/tui/view_test.go @@ -0,0 +1,286 @@ +package tui + +import ( + "regexp" + "strings" + "testing" + "time" + + "github.com/yeniklas/terdut-tui/internal/api" +) + +// ansi matches the escape sequences lipgloss emits when it decides the output +// supports colour, so assertions can be made against the text alone. +var ansi = regexp.MustCompile(`\x1b\[[0-9;]*m`) + +func plain(s string) string { return ansi.ReplaceAllString(s, "") } + +func mustContain(t *testing.T, got string, wants ...string) { + t.Helper() + got = plain(got) + for _, w := range wants { + if !strings.Contains(got, w) { + t.Errorf("expected output to contain %q\n--- got ---\n%s", w, got) + } + } +} + +func TestIncidentDetail_RendersTheWholeStory(t *testing.T) { + now := time.Now() + ackID := int64(1) + alertID := int64(3) + inc := api.Incident{ + ID: 1, Title: "DiskFull (namespace=prod)", Status: api.StatusAcknowledged, + Severity: "critical", + GroupLabels: map[string]string{"alertname": "DiskFull", "namespace": "prod"}, + TriggeredAt: now.Add(-2 * time.Hour), + AssignedTo: "admin", AcknowledgedByID: &ackID, AcknowledgedBy: "admin", + AcknowledgedAt: &now, + Alerts: []api.Alert{ + {ID: 3, Name: "DiskFull", Status: "firing", + Labels: map[string]string{"instance": "node-1"}, ReceivedAt: now}, + {ID: 4, Name: "DiskFull", Status: "resolved", + Labels: map[string]string{"instance": "node-2"}, ReceivedAt: now}, + }, + } + timeline := []api.IncidentEvent{ + {Type: api.EventTriggered, CreatedAt: now}, + {Type: api.EventAssigned, Username: "admin", CreatedAt: now}, + {Type: api.EventAlertAdded, AlertID: &alertID, CreatedAt: now}, + {Type: api.EventAcknowledged, Username: "admin", CreatedAt: now}, + {Type: api.EventNote, Username: "admin", Detail: "draining node-2", CreatedAt: now}, + } + + out := buildIncidentDetailContent(inc, timeline, -1, 110) + mustContain(t, out, + "DiskFull (namespace=prod)", "ACKNOWLEDGED", "CRITICAL", + "Assigned:", "admin", + "Grouped By", "namespace", "prod", + "Alerts (2)", "node-1", "node-2", + "Timeline (5 events, 1 notes)", + "Incident opened", "Assigned to admin", "Alert #3 joined", "Acknowledged by admin", + "admin wrote", "draining node-2", + ) +} + +func TestIncidentDetail_ShowsSnooze(t *testing.T) { + future := time.Now().Add(2 * time.Hour) + inc := api.Incident{ + Title: "Noisy", Status: api.StatusTriggered, + TriggeredAt: time.Now(), SnoozedUntil: &future, + } + // The exact remaining time is humanUntil's business, not this test's — a few + // microseconds of elapsed clock turn "in 2h" into "in 1h 59m". + mustContain(t, buildIncidentDetailContent(inc, nil, -1, 110), + "TRIGGERED (snoozed)", "Snoozed:", "until", "in 1h") +} + +// An expired snooze is not a snooze, so it must not be reported as one. +func TestIncidentDetail_HidesExpiredSnooze(t *testing.T) { + past := time.Now().Add(-time.Hour) + inc := api.Incident{ + Title: "Noisy", Status: api.StatusTriggered, + TriggeredAt: time.Now(), SnoozedUntil: &past, + } + if strings.Contains(plain(buildIncidentDetailContent(inc, nil, -1, 110)), "Snoozed:") { + t.Error("an expired snooze should not be rendered") + } +} + +func TestIncidentDetail_ShowsResolutionSource(t *testing.T) { + now := time.Now() + source := "manual" + inc := api.Incident{ + Title: "Done", Status: api.StatusResolved, TriggeredAt: now.Add(-time.Hour), + ResolvedAt: &now, ResolutionSource: &source, + } + mustContain(t, buildIncidentDetailContent(inc, nil, -1, 110), + "RESOLVED", "Resolved:", "manual") +} + +func TestIncidentDetail_UnassignedAndUnacknowledged(t *testing.T) { + inc := api.Incident{Title: "Fresh", Status: api.StatusTriggered, TriggeredAt: time.Now()} + mustContain(t, buildIncidentDetailContent(inc, nil, -1, 110), "nobody", "not acknowledged") +} + +func TestIncidentDetail_EmptyTimeline(t *testing.T) { + inc := api.Incident{Title: "Fresh", Status: api.StatusTriggered, TriggeredAt: time.Now()} + mustContain(t, buildIncidentDetailContent(inc, nil, -1, 110), "Nothing recorded yet") +} + +func TestIncidentDetail_MarksSelectedNote(t *testing.T) { + now := time.Now() + timeline := []api.IncidentEvent{ + {Type: api.EventNote, Username: "admin", Detail: "first", CreatedAt: now}, + {Type: api.EventNote, Username: "alice", Detail: "second", CreatedAt: now}, + } + inc := api.Incident{Title: "X", Status: api.StatusTriggered, TriggeredAt: now} + + out := plain(buildIncidentDetailContent(inc, timeline, 1, 110)) + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, "alice") && !strings.HasPrefix(line, "> ") { + t.Errorf("expected the selected note marked, got %q", line) + } + if strings.Contains(line, "admin wrote") && strings.HasPrefix(line, "> ") { + t.Errorf("expected the unselected note unmarked, got %q", line) + } + } +} + +func TestIncidentStatusLabel(t *testing.T) { + future := time.Now().Add(time.Hour) + tests := []struct { + name string + inc api.Incident + want string + }{ + {"triggered", api.Incident{Status: api.StatusTriggered}, "● TRIGGERED"}, + {"acknowledged", api.Incident{Status: api.StatusAcknowledged}, "◐ ACKNOWLEDGED"}, + {"resolved", api.Incident{Status: api.StatusResolved}, "✓ RESOLVED"}, + {"snoozed", api.Incident{Status: api.StatusTriggered, SnoozedUntil: &future}, + "● TRIGGERED (snoozed)"}, + // A status this client does not know about still has to render. + {"unknown", api.Incident{Status: "escalated"}, "ESCALATED"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := incidentStatusLabel(tt.inc); got != tt.want { + t.Errorf("expected %q, got %q", tt.want, got) + } + }) + } +} + +// The server may add event types after this client ships. An unknown one must +// still appear on the timeline rather than silently vanishing. +func TestEventLabel_UnknownTypeFallsBackToItsName(t *testing.T) { + got := eventLabel(api.IncidentEvent{Type: "escalated", Detail: "to sre-oncall"}) + mustContain(t, got, "escalated", "to sre-oncall") +} + +func TestEventLabel_KnownTypes(t *testing.T) { + alertID := int64(9) + tests := []struct { + event api.IncidentEvent + want string + }{ + {api.IncidentEvent{Type: api.EventTriggered}, "Incident opened"}, + {api.IncidentEvent{Type: api.EventAlertAdded, AlertID: &alertID}, "Alert #9 joined"}, + {api.IncidentEvent{Type: api.EventAlertResolved, AlertID: &alertID}, "Alert #9 resolved"}, + {api.IncidentEvent{Type: api.EventAcknowledged, Username: "bo"}, "Acknowledged by bo"}, + {api.IncidentEvent{Type: api.EventAssigned, Username: "bo"}, "Assigned to bo"}, + {api.IncidentEvent{Type: api.EventSnoozed, Detail: "2026-08-01T00:00:00Z"}, + "Snoozed until 2026-08-01T00:00:00Z"}, + {api.IncidentEvent{Type: api.EventResolved, Username: "bo"}, "Resolved by bo"}, + // No user means the server closed it via the alert cascade. + {api.IncidentEvent{Type: api.EventResolved}, "all alerts stopped firing"}, + } + for _, tt := range tests { + t.Run(tt.event.Type, func(t *testing.T) { + mustContain(t, eventLabel(tt.event), tt.want) + }) + } +} + +func TestAlertDetail_SaysItIsReadOnlyAndLinksTheIncident(t *testing.T) { + now := time.Now() + id := int64(7) + alert := api.Alert{ + ID: 3, Name: "DiskFull", Status: "firing", StartsAt: now.Add(-time.Hour), + ReceivedAt: now, IncidentID: &id, + Labels: map[string]string{"instance": "node-1", "severity": "critical"}, + Annotations: map[string]string{"summary": "disk 90%"}, + } + mustContain(t, buildAlertDetailContent(alert, 110), + "DiskFull", "FIRING", "Incident:", "#7", "press i to open it", + "instance", "node-1", "summary", "disk 90%", + "Alerts are read-only") +} + +func TestAlertDetail_NoIncident(t *testing.T) { + alert := api.Alert{ID: 3, Name: "Orphan", Status: "resolved", ReceivedAt: time.Now()} + mustContain(t, buildAlertDetailContent(alert, 110), "Incident:", "none") +} + +func TestAlertDetail_ShowsResolutionSource(t *testing.T) { + source := "expiry" + alert := api.Alert{Name: "Gone", Status: "resolved", ReceivedAt: time.Now(), + ResolutionSource: &source} + mustContain(t, buildAlertDetailContent(alert, 110), "RESOLVED", "expiry") +} + +// Null MTTA means nothing has been acknowledged, which is a different claim +// from an instant response. +func TestStats_RendersDashForMissingAverages(t *testing.T) { + stats := &api.IncidentStats{Total: 2, Triggered: 2} + out := buildStatsContent(stats, nil, nil, nil, 110) + mustContain(t, out, "Incident Response", "Mean time to acknowledge", "—", + "nothing has been acknowledged or resolved yet") +} + +func TestStats_RendersAverages(t *testing.T) { + mtta, mttr := 150.0, 3600.0 + stats := &api.IncidentStats{Total: 3, Resolved: 1, MTTASeconds: &mtta, MTTRSeconds: &mttr} + out := buildStatsContent(stats, []api.TopAlert{{Name: "DiskFull", Count: 4}}, nil, nil, 110) + mustContain(t, out, "2m", "1h", "Top Alerts", "DiskFull") +} + +func TestStats_HandlesNoIncidentData(t *testing.T) { + mustContain(t, buildStatsContent(nil, nil, nil, nil, 110), "Incident Response", "No data") +} + +func TestView_TabsAndDashboardRender(t *testing.T) { + m := sized() + m.incidents = []api.Incident{{ + ID: 1, Title: "DiskFull", Status: api.StatusTriggered, Severity: "critical", + AssignedTo: "admin", TriggeredAt: time.Now(), + }} + m.incidentStats = &api.IncidentStats{Triggered: 1} + m.rebuildIncidentTable() + + mustContain(t, m.View(), + "Incidents", "Alerts", "Archived", "Schedule", "Users", + "Triggered: 1", "filter: open", + "DiskFull", "critical", "admin", + "enter·detail") +} + +func TestView_EmptyStates(t *testing.T) { + m := sized() + m.loading = false + mustContain(t, m.View(), "No open incidents.") + + m.activeSection = sectionArchived + mustContain(t, m.View(), "No archived incidents.") +} + +func TestView_ConnectionError(t *testing.T) { + m := sized() + m.connected = false + m.err = errFixture{} + mustContain(t, m.View(), "Error:", "Press r to retry") +} + +// The footer is the only place the terminal states are explained, so the +// destructive one has to be visible before it is pressed. +func TestFooter_IncidentDetailOffersResolveOnlyWhileOpen(t *testing.T) { + open := onIncident(openIncidentFixture(), nil) + mustContain(t, open.renderFooter(), "R·resolve", "z·snooze", "a·ack") + + closed := onIncident(resolvedIncidentFixture(), nil) + if strings.Contains(plain(closed.renderFooter()), "R·resolve") { + t.Error("a resolved incident should not offer resolve") + } + mustContain(t, closed.renderFooter(), "x·archive", "c·note") +} + +func TestView_ZeroWidthRendersNothing(t *testing.T) { + m := NewModel(nil, "http://test", time.Minute) + if m.View() != "" { + t.Error("expected no output before the first window size message") + } +} + +type errFixture struct{} + +func (errFixture) Error() string { return "connection refused" }