diff --git a/cmd/root.go b/cmd/root.go index 3be8661..28c5822 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -194,35 +194,63 @@ func initConfig() { } } -func configureLogging() { - log.SetPrefix("srepd") +// LogDestination represents where logs should be written +type LogDestination int - switch runtime.GOOS { +const ( + LogToJournal LogDestination = iota + LogToFile + LogToStderr +) + +// determineLogDestination returns the appropriate log destination based on OS and config +func determineLogDestination(goos string, logToJournal bool, journalEnabled bool) (LogDestination, string) { + switch goos { case "linux": - // Check if running under systemd - if journal.Enabled() { - log.SetOutput(journalWriter{}) - log.Info("Logging to systemd journal") - return + if logToJournal { + if journalEnabled { + return LogToJournal, "" + } + return LogToFile, "/var/log/srepd.log" } - - // Fallback to /var/log/srepd.log for non-systemd Linux - logFile := "/var/log/srepd.log" - setupFileLogging(logFile) - log.Info("Logging to /var/log/srepd.log") + // User explicitly wants file logging + return LogToFile, "~/.config/srepd/debug.log" case "darwin": - // macOS: Log to ~/Library/Logs/srepd.log - home, err := os.UserHomeDir() - if err != nil { - log.Fatal("Failed to get user home directory:", err) - } - logFile := home + "/Library/Logs/srepd.log" - setupFileLogging(logFile) - log.Info("Logging to ~/Library/Logs/srepd.log") + return LogToFile, "~/Library/Logs/srepd.log" default: - // Default fallback for other OSes + return LogToStderr, "" + } +} + +func configureLogging() { + log.SetPrefix("srepd") + + // Check if user wants to log to journal (default: true) + viper.SetDefault("log_to_journal", true) + logToJournal := viper.GetBool("log_to_journal") + + dest, logPath := determineLogDestination(runtime.GOOS, logToJournal, journal.Enabled()) + + switch dest { + case LogToJournal: + log.SetOutput(journalWriter{}) + log.Info("Logging to systemd journal") + + case LogToFile: + // Expand home directory if needed + if strings.HasPrefix(logPath, "~/") { + home, err := os.UserHomeDir() + if err != nil { + log.Fatal("Failed to get user home directory:", err) + } + logPath = home + logPath[1:] + } + setupFileLogging(logPath) + log.Info("Logging to " + logPath) + + case LogToStderr: log.SetOutput(os.Stderr) log.Warn("Unsupported OS: logging to stderr") } diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 0000000..bd70878 --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,91 @@ +package cmd + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDetermineLogDestination(t *testing.T) { + tests := []struct { + name string + goos string + logToJournal bool + journalEnabled bool + expectedDest LogDestination + expectedPath string + }{ + { + name: "Linux with journal enabled and log_to_journal=true", + goos: "linux", + logToJournal: true, + journalEnabled: true, + expectedDest: LogToJournal, + expectedPath: "", + }, + { + name: "Linux with journal disabled and log_to_journal=true", + goos: "linux", + logToJournal: true, + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "/var/log/srepd.log", + }, + { + name: "Linux with log_to_journal=false (user wants file logging)", + goos: "linux", + logToJournal: false, + journalEnabled: true, // Journal enabled but user wants file + expectedDest: LogToFile, + expectedPath: "~/.config/srepd/debug.log", + }, + { + name: "Linux with log_to_journal=false and journal disabled", + goos: "linux", + logToJournal: false, + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "~/.config/srepd/debug.log", + }, + { + name: "macOS always logs to file", + goos: "darwin", + logToJournal: true, // Ignored on macOS + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "~/Library/Logs/srepd.log", + }, + { + name: "macOS with log_to_journal=false", + goos: "darwin", + logToJournal: false, + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "~/Library/Logs/srepd.log", + }, + { + name: "Unsupported OS logs to stderr", + goos: "windows", + logToJournal: true, + journalEnabled: false, + expectedDest: LogToStderr, + expectedPath: "", + }, + { + name: "Unknown OS logs to stderr", + goos: "freebsd", + logToJournal: false, + journalEnabled: false, + expectedDest: LogToStderr, + expectedPath: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dest, path := determineLogDestination(tt.goos, tt.logToJournal, tt.journalEnabled) + assert.Equal(t, tt.expectedDest, dest, "Log destination mismatch") + assert.Equal(t, tt.expectedPath, path, "Log path mismatch") + }) + } +} diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index 03813dc..fa00f88 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -79,8 +79,9 @@ func getIncident(p *pd.Config, id string) tea.Cmd { // gotIncidentAlertsMsg is a message that contains the fetched incident alerts type gotIncidentAlertsMsg struct { - alerts []pagerduty.IncidentAlert - err error + incidentID string + alerts []pagerduty.IncidentAlert + err error } // getIncidentAlerts returns a command that fetches the alerts for the given incident @@ -90,14 +91,15 @@ func getIncidentAlerts(p *pd.Config, id string) tea.Cmd { return setStatusMsg{nilIncidentMsg} } a, err := pd.GetAlerts(p.Client, id, pagerduty.ListIncidentAlertsOptions{}) - return gotIncidentAlertsMsg{a, err} + return gotIncidentAlertsMsg{incidentID: id, alerts: a, err: err} } } // gotIncidentNotesMsg is a message that contains the fetched incident notes type gotIncidentNotesMsg struct { - notes []pagerduty.IncidentNote - err error + incidentID string + notes []pagerduty.IncidentNote + err error } // getIncidentNotes returns a command that fetches the notes for the given incident @@ -107,7 +109,7 @@ func getIncidentNotes(p *pd.Config, id string) tea.Cmd { return setStatusMsg{nilIncidentMsg} } n, err := pd.GetNotes(p.Client, id) - return gotIncidentNotesMsg{n, err} + return gotIncidentNotesMsg{incidentID: id, notes: n, err: err} } } @@ -327,13 +329,6 @@ func openBrowserCmd(browser []string, url string) tea.Cmd { c := exec.Command(browser[0], args...) log.Debug(fmt.Sprintf("tui.openBrowserCmd(): %v", c.String())) - stderr, pipeErr := c.StderrPipe() - if pipeErr != nil { - log.Debug(fmt.Sprintf("tui.openBrowserCmd(): %v", pipeErr.Error())) - return func() tea.Msg { - return browserFinishedMsg{err: pipeErr} - } - } err := c.Start() if err != nil { @@ -343,20 +338,8 @@ func openBrowserCmd(browser []string, url string) tea.Cmd { } } - out, err := io.ReadAll(stderr) - if err != nil { - log.Debug(fmt.Sprintf("tui.openBrowserCmd(): %v", err.Error())) - return func() tea.Msg { - return browserFinishedMsg{err} - } - } - - if len(out) > 0 { - log.Debug(fmt.Sprintf("tui.openBrowserCmd(): error: %s", out)) - return func() tea.Msg { - return browserFinishedMsg{fmt.Errorf("%s", out)} - } - } + // Browser opened successfully - don't wait for it or check stderr + // Browsers write warnings/info to stderr that aren't actual errors return func() tea.Msg { return browserFinishedMsg{} @@ -607,12 +590,32 @@ var errSilenceIncidentInvalidArgs = errors.New("silenceIncidents: invalid argume func silenceIncidents(incidents []pagerduty.Incident, policy *pagerduty.EscalationPolicy, level uint) tea.Cmd { // SilenceIncidents doesn't have it's own "silencedIncidentsMessage" because it's really just a re-escalation - log.Printf("silence requested for incident(s) %v; reassigning to %s(%s), level %d", incidents, policy.Name, policy.ID, level) - return func() tea.Msg { - if len(incidents) == 0 || policy.Name == "" || level == 0 { + if policy == nil { + log.Debug("silenceIncidents: nil escalation policy provided") + return func() tea.Msg { + return errMsg{errSilenceIncidentInvalidArgs} + } + } + + if len(incidents) == 0 { + log.Debug("silenceIncidents: no incidents provided") + return func() tea.Msg { + return errMsg{errSilenceIncidentInvalidArgs} + } + } + + if policy.Name == "" || policy.ID == "" || level == 0 { + log.Debug("silenceIncidents: invalid arguments", "incident(s) count: %d; policy: %s(%s), level: %d", len(incidents), policy.Name, policy.ID, level) + return func() tea.Msg { return errMsg{errSilenceIncidentInvalidArgs} } - // + } + + incidentIDs := getIDsFromIncidents(incidents) + + log.Printf("silence requested for incident(s) %v; reassigning to %s(%s), level %d", incidentIDs, policy.Name, policy.ID, level) + + return func() tea.Msg { return reEscalateIncidentsMsg{incidents, policy, level} } } @@ -734,3 +737,11 @@ func runScheduledJobs(m *model) []tea.Cmd { } return cmds } + +func getIDsFromIncidents(incidents []pagerduty.Incident) []string { + var ids []string + for _, i := range incidents { + ids = append(ids, i.ID) + } + return ids +} diff --git a/pkg/tui/commands_test.go b/pkg/tui/commands_test.go index 17f510a..5acdff2 100644 --- a/pkg/tui/commands_test.go +++ b/pkg/tui/commands_test.go @@ -133,6 +133,7 @@ func TestGetIncidentAlerts(t *testing.T) { mockConfig := &pd.Config{ Client: &pd.MockPagerDutyClient{}, } + testID := rand.ID("Q") tests := []struct { name string config *pd.Config @@ -148,8 +149,9 @@ func TestGetIncidentAlerts(t *testing.T) { { name: "return gotIncidentAlertsMsg if incident id is provided", config: mockConfig, - id: rand.ID("Q"), + id: testID, expected: gotIncidentAlertsMsg{ + incidentID: testID, alerts: []pagerduty.IncidentAlert{ {APIObject: pagerduty.APIObject{ID: "QABCDEFG1234567"}}, {APIObject: pagerduty.APIObject{ID: "QABCDEFG7654321"}}, @@ -162,8 +164,9 @@ func TestGetIncidentAlerts(t *testing.T) { config: mockConfig, id: "err", // "err" signals the mock client to produce a mock error expected: gotIncidentAlertsMsg{ - alerts: nil, - err: fmt.Errorf("pd.GetAlerts(): failed to get alerts for incident `%v`: %v", "err", pd.ErrMockError), + incidentID: "err", + alerts: nil, + err: fmt.Errorf("pd.GetAlerts(): failed to get alerts for incident `%v`: %v", "err", pd.ErrMockError), }, }, } @@ -181,6 +184,7 @@ func TestGetIncidentNotes(t *testing.T) { mockConfig := &pd.Config{ Client: &pd.MockPagerDutyClient{}, } + testID := rand.ID("Q") tests := []struct { name string config *pd.Config @@ -196,8 +200,9 @@ func TestGetIncidentNotes(t *testing.T) { { name: "return gotIncidentNotesMsg if incident id is provided", config: mockConfig, - id: rand.ID("Q"), + id: testID, expected: gotIncidentNotesMsg{ + incidentID: testID, notes: []pagerduty.IncidentNote{ {ID: "QABCDEFG1234567"}, {ID: "QABCDEFG7654321"}, @@ -210,8 +215,9 @@ func TestGetIncidentNotes(t *testing.T) { config: mockConfig, id: "err", // "err" signals the mock client to produce a mock error expected: gotIncidentNotesMsg{ - notes: []pagerduty.IncidentNote{}, - err: fmt.Errorf("pd.GetNotes(): failed to get incident notes `%v`: %v", "err", pd.ErrMockError), + incidentID: "err", + notes: []pagerduty.IncidentNote{}, + err: fmt.Errorf("pd.GetNotes(): failed to get incident notes `%v`: %v", "err", pd.ErrMockError), }, }, } @@ -357,3 +363,46 @@ func TestGetEscalationPolicyKey(t *testing.T) { }) } } + +func TestOpenBrowserCmd(t *testing.T) { + tests := []struct { + name string + browser []string + url string + expectError bool + errorContains string + }{ + { + name: "successful command execution", + browser: []string{"echo", "browser"}, + url: "https://example.com", + expectError: false, + }, + { + name: "command not found returns error", + browser: []string{"nonexistent-browser-command-xyz"}, + url: "https://example.com", + expectError: true, + errorContains: "not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := openBrowserCmd(tt.browser, tt.url) + result := cmd() + + msg, ok := result.(browserFinishedMsg) + assert.True(t, ok, "Expected browserFinishedMsg type") + + if tt.expectError { + assert.NotNil(t, msg.err, "Expected error but got nil") + if tt.errorContains != "" { + assert.Contains(t, msg.err.Error(), tt.errorContains, "Error message mismatch") + } + } else { + assert.Nil(t, msg.err, "Expected no error but got: %v", msg.err) + } + }) + } +} diff --git a/pkg/tui/model.go b/pkg/tui/model.go index 75b59ea..adbc411 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -14,6 +14,17 @@ import ( "github.com/clcollins/srepd/pkg/pd" ) +// cachedIncidentData stores fetched incident data for reuse +type cachedIncidentData struct { + incident *pagerduty.Incident + notes []pagerduty.IncidentNote + alerts []pagerduty.IncidentAlert + dataLoaded bool + notesLoaded bool + alertsLoaded bool + lastFetched time.Time +} + var initialScheduledJobs = []*scheduledJob{ { jobMsg: func() tea.Msg { return PollIncidentsMsg{} }, @@ -42,6 +53,14 @@ type model struct { selectedIncidentNotes []pagerduty.IncidentNote selectedIncidentAlerts []pagerduty.IncidentAlert + // Loading state tracking - enables progressive rendering and action guards + incidentDataLoaded bool + incidentNotesLoaded bool + incidentAlertsLoaded bool + + // Incident data cache - stores fetched data for reuse and pre-fetching + incidentCache map[string]*cachedIncidentData + scheduledJobs []*scheduledJob autoAcknowledge bool @@ -71,7 +90,9 @@ func InitialModel( input: newTextInput(), incidentViewer: newIncidentViewer(), status: "", + incidentCache: make(map[string]*cachedIncidentData), scheduledJobs: append([]*scheduledJob{}, initialScheduledJobs...), + autoRefresh: true, // Start watching for updates on startup } // This is an ugly way to handle this error @@ -103,6 +124,10 @@ func (m *model) clearSelectedIncident(reason interface{}) { m.selectedIncidentNotes = nil m.selectedIncidentAlerts = nil m.viewingIncident = false + // Clear loading flags + m.incidentDataLoaded = false + m.incidentNotesLoaded = false + m.incidentAlertsLoaded = false log.Debug("clearSelectedIncident", "selectedIncident", m.selectedIncident, "cleared", true, "reason", reason) } diff --git a/pkg/tui/model_test.go b/pkg/tui/model_test.go new file mode 100644 index 0000000..59f4ff9 --- /dev/null +++ b/pkg/tui/model_test.go @@ -0,0 +1,267 @@ +package tui + +import ( + "testing" + + "github.com/PagerDuty/go-pagerduty" + tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/assert" +) + +func TestLoadingStateTracking(t *testing.T) { + tests := []struct { + name string + msg tea.Msg + initialDataLoaded bool + initialNotesLoaded bool + initialAlertsLoaded bool + expectedDataLoaded bool + expectedNotesLoaded bool + expectedAlertsLoaded bool + setupSelectedIncident bool + }{ + { + name: "gotIncidentMsg sets incidentDataLoaded to true", + msg: gotIncidentMsg{ + incident: &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "Q123"}, + }, + err: nil, + }, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: true, + expectedNotesLoaded: false, + expectedAlertsLoaded: false, + setupSelectedIncident: true, + }, + { + name: "gotIncidentNotesMsg sets incidentNotesLoaded to true", + msg: gotIncidentNotesMsg{ + incidentID: "Q123", + notes: []pagerduty.IncidentNote{ + {ID: "N123", Content: "Test note"}, + }, + err: nil, + }, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: false, + expectedNotesLoaded: true, + expectedAlertsLoaded: false, + setupSelectedIncident: true, + }, + { + name: "gotIncidentAlertsMsg sets incidentAlertsLoaded to true", + msg: gotIncidentAlertsMsg{ + incidentID: "Q123", + alerts: []pagerduty.IncidentAlert{ + {APIObject: pagerduty.APIObject{ID: "A123"}}, + }, + err: nil, + }, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: false, + expectedNotesLoaded: false, + expectedAlertsLoaded: true, + setupSelectedIncident: true, + }, + { + name: "clearSelectedIncidentsMsg clears all loading flags", + msg: clearSelectedIncidentsMsg("test"), + initialDataLoaded: true, + initialNotesLoaded: true, + initialAlertsLoaded: true, + expectedDataLoaded: false, + expectedNotesLoaded: false, + expectedAlertsLoaded: false, + setupSelectedIncident: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := model{ + incidentDataLoaded: tt.initialDataLoaded, + incidentNotesLoaded: tt.initialNotesLoaded, + incidentAlertsLoaded: tt.initialAlertsLoaded, + incidentCache: make(map[string]*cachedIncidentData), + } + + if tt.setupSelectedIncident { + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "Q123"}, + } + } + + result, _ := m.Update(tt.msg) + m = result.(model) + + assert.Equal(t, tt.expectedDataLoaded, m.incidentDataLoaded, "incidentDataLoaded mismatch") + assert.Equal(t, tt.expectedNotesLoaded, m.incidentNotesLoaded, "incidentNotesLoaded mismatch") + assert.Equal(t, tt.expectedAlertsLoaded, m.incidentAlertsLoaded, "incidentAlertsLoaded mismatch") + }) + } +} + +func TestActionGuards(t *testing.T) { + tests := []struct { + name string + keyMsg tea.KeyMsg + incidentDataLoaded bool + incidentAlertsLoaded bool + expectedAction bool + expectedStatus string + }{ + { + name: "Note action blocked when data not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'n'}}, + incidentDataLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident details, please wait...", + }, + { + name: "Note action allowed when data loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'n'}}, + incidentDataLoaded: true, + expectedAction: true, + expectedStatus: "", + }, + { + name: "Login action blocked when alerts not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, + incidentAlertsLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident alerts, please wait...", + }, + { + name: "Login action allowed when alerts loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, + incidentAlertsLoaded: true, + expectedAction: true, + expectedStatus: "", + }, + { + name: "Open action blocked when data not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}, + incidentDataLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident details, please wait...", + }, + { + name: "Open action allowed when data loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}, + incidentDataLoaded: true, + expectedAction: true, + expectedStatus: "", + }, + { + name: "Acknowledge action always allowed (only needs ID)", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}}, + incidentDataLoaded: false, + expectedAction: true, + expectedStatus: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := model{ + viewingIncident: true, + incidentDataLoaded: tt.incidentDataLoaded, + incidentAlertsLoaded: tt.incidentAlertsLoaded, + selectedIncident: &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "Q123"}, + }, + incidentCache: make(map[string]*cachedIncidentData), + } + + initialStatus := m.status + result, cmd := m.Update(tt.keyMsg) + m = result.(model) + + if !tt.expectedAction { + // Action should be blocked - no command returned + assert.Nil(t, cmd, "Expected no command when action is blocked") + assert.Equal(t, tt.expectedStatus, m.status, "Status message mismatch") + } else if tt.keyMsg.Runes[0] != 'a' { // Skip acknowledge since it generates a different message + // Action should be allowed - command returned + assert.NotNil(t, cmd, "Expected command when action is allowed") + // Status should not be the "waiting" messages + assert.NotEqual(t, "Loading incident details, please wait...", m.status, "Should not show data waiting message") + assert.NotEqual(t, "Loading incident alerts, please wait...", m.status, "Should not show alerts waiting message") + } + + // Reset status for next iteration + m.status = initialStatus + }) + } +} + +func TestProgressiveRendering(t *testing.T) { + tests := []struct { + name string + incidentNotesLoaded bool + incidentAlertsLoaded bool + expectedNotesContent string + expectedAlertsMarker string + }{ + { + name: "Shows loading message for notes when not loaded", + incidentNotesLoaded: false, + incidentAlertsLoaded: true, + expectedNotesContent: "Loading notes...", + expectedAlertsMarker: "", + }, + { + name: "Shows loading message for alerts when not loaded", + incidentNotesLoaded: true, + incidentAlertsLoaded: false, + expectedNotesContent: "", + expectedAlertsMarker: "Loading alerts...", + }, + { + name: "Shows both loading messages when neither loaded", + incidentNotesLoaded: false, + incidentAlertsLoaded: false, + expectedNotesContent: "Loading notes...", + expectedAlertsMarker: "Loading alerts...", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + incident := &pagerduty.Incident{} + incident.ID = "Q123" + incident.Summary = "Test Incident" + incident.HTMLURL = "https://example.com/incident" + incident.Title = "Test Incident Title" + incident.Status = "triggered" + incident.Urgency = "high" + // Set Service summary for template + incident.Service.Summary = "test-service" + + m := model{ + selectedIncident: incident, + incidentNotesLoaded: tt.incidentNotesLoaded, + incidentAlertsLoaded: tt.incidentAlertsLoaded, + incidentCache: make(map[string]*cachedIncidentData), + } + + content, err := m.template() + assert.NoError(t, err, "Template should render without error") + + if tt.expectedNotesContent != "" { + assert.Contains(t, content, tt.expectedNotesContent, "Should contain notes loading message") + } + + if tt.expectedAlertsMarker != "" { + assert.Contains(t, content, tt.expectedAlertsMarker, "Should contain alerts loading message") + } + }) + } +} diff --git a/pkg/tui/msgHandlers.go b/pkg/tui/msgHandlers.go index 887386f..90a49fc 100644 --- a/pkg/tui/msgHandlers.go +++ b/pkg/tui/msgHandlers.go @@ -122,6 +122,13 @@ func (m model) keyMsgHandler(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } + // Commands for any focus mode + if key.Matches(msg.(tea.KeyMsg), defaultKeyMap.Input) { + return m, tea.Sequence( + m.input.Focus(), + ) + } + // Default commands for the table view switch { case m.err != nil: @@ -175,11 +182,6 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { case key.Matches(msg, defaultKeyMap.Bottom): m.table.GotoBottom() - case key.Matches(msg, defaultKeyMap.Input): - return m, tea.Sequence( - m.input.Focus(), - ) - case key.Matches(msg, defaultKeyMap.Team): m.teamMode = !m.teamMode log.Debug("switchTableFocusMode", "teamMode", m.teamMode) @@ -195,12 +197,57 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { // "waitForSelectedIncidentsThen..." functions are used to wait for the selected incident // to be retrieved from PagerDuty case key.Matches(msg, defaultKeyMap.Enter): - m.viewingIncident = true - return m, doIfIncidentSelected(&m, func() tea.Msg { - return waitForSelectedIncidentThenDoMsg{ - action: func() tea.Msg { return renderIncidentMsg("render") }, msg: "render", + // Check if we have cached data for this incident + if cached, exists := m.incidentCache[incidentID]; exists { + // Use cached data immediately + if cached.incident != nil { + m.selectedIncident = cached.incident + m.incidentDataLoaded = cached.dataLoaded + } else { + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ + ID: incidentID, + Summary: "Loading incident details...", + }, + } + m.incidentDataLoaded = false } - }) + + if cached.notes != nil { + m.selectedIncidentNotes = cached.notes + m.incidentNotesLoaded = cached.notesLoaded + } else { + m.incidentNotesLoaded = false + } + + if cached.alerts != nil { + m.selectedIncidentAlerts = cached.alerts + m.incidentAlertsLoaded = cached.alertsLoaded + } else { + m.incidentAlertsLoaded = false + } + } else { + // No cache - show loading placeholder + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ + ID: incidentID, + Summary: "Loading incident details...", + }, + } + m.incidentDataLoaded = false + m.incidentNotesLoaded = false + m.incidentAlertsLoaded = false + } + + m.viewingIncident = true + // Reset viewport to top when opening incident + m.incidentViewer.GotoTop() + // Render with whatever data we have (cached or loading placeholder) + // Then refresh from PagerDuty in the background + return m, tea.Sequence( + func() tea.Msg { return renderIncidentMsg("Render incident: " + incidentID) }, + func() tea.Msg { return getIncidentMsg(incidentID) }, + ) case key.Matches(msg, defaultKeyMap.Silence): return m, doIfIncidentSelected(&m, tea.Sequence( @@ -309,22 +356,42 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { case key.Matches(msg, defaultKeyMap.Back): m.clearSelectedIncident(msg.String() + " (back)") + case key.Matches(msg, defaultKeyMap.Refresh): + return m, func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) } + case key.Matches(msg, defaultKeyMap.Ack): return m, func() tea.Msg { return acknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } + case key.Matches(msg, defaultKeyMap.UnAck): + return m, func() tea.Msg { return unAcknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } + case key.Matches(msg, defaultKeyMap.Silence): return m, func() tea.Msg { return silenceSelectedIncidentMsg{} } - case key.Matches(msg, defaultKeyMap.Refresh): - return m, func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) } + case key.Matches(msg, defaultKeyMap.Note): + // Note template requires full incident data (HTMLURL, Title, Service.Summary) + if !m.incidentDataLoaded { + m.setStatus("Loading incident details, please wait...") + return m, nil + } + return m, func() tea.Msg { return parseTemplateForNoteMsg("add note") } case key.Matches(msg, defaultKeyMap.Login): - return m, tea.Sequence( - func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) }, - func() tea.Msg { - return waitForSelectedIncidentThenDoMsg{action: func() tea.Msg { return loginMsg("login") }, msg: "wait"} - }, - ) + // Login requires alerts to extract cluster_id + if !m.incidentAlertsLoaded { + m.setStatus("Loading incident alerts, please wait...") + return m, nil + } + return m, func() tea.Msg { return loginMsg("login") } + + case key.Matches(msg, defaultKeyMap.Open): + // Browser open requires HTMLURL from full incident data + if !m.incidentDataLoaded { + m.setStatus("Loading incident details, please wait...") + return m, nil + } + return m, func() tea.Msg { return openBrowserMsg("incident") } + } } diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 62dbd0b..b49460a 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -106,8 +106,15 @@ func filterMsgContent(msg tea.Msg) tea.Msg { // return m, func() tea.Msg { getIncident(m.config, msg.incident.ID) } func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { msgType := reflect.TypeOf(msg) - // TickMsg are not helpful for logging - if msgType != reflect.TypeOf(TickMsg{}) { + // TickMsg and arrow key messages are not helpful for logging + shouldLog := msgType != reflect.TypeOf(TickMsg{}) + if keyMsg, ok := msg.(tea.KeyMsg); ok && shouldLog { + // Skip logging for arrow keys used in scrolling + if keyMsg.Type == tea.KeyUp || keyMsg.Type == tea.KeyDown { + shouldLog = false + } + } + if shouldLog { log.Debug("Update", msgType, filterMsgContent(msg)) } @@ -146,38 +153,82 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.setStatus(fmt.Sprintf("getting details for incident %v...", msg)) - cmds = append(cmds, getIncident(m.config, string(msg))) + id := string(msg) + cmds = append(cmds, + getIncident(m.config, id), + getIncidentAlerts(m.config, id), + getIncidentNotes(m.config, id), + ) // Set the selected incident to the incident returned from the getIncident command case gotIncidentMsg: if msg.err != nil { + m.selectedIncident = nil + m.viewingIncident = false return m, func() tea.Msg { return errMsg{msg.err} } } - m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) - m.selectedIncident = msg.incident - return m, tea.Batch( - getIncidentAlerts(m.config, msg.incident.ID), - getIncidentNotes(m.config, msg.incident.ID), - ) + // Update cache with fetched incident data + if cached, exists := m.incidentCache[msg.incident.ID]; exists { + log.Debug("Update", "gotIncidentMsg", "refreshing cached incident data", "incident", msg.incident.ID) + cached.incident = msg.incident + cached.dataLoaded = true + cached.lastFetched = time.Now() + } else { + log.Debug("Update", "gotIncidentMsg", "caching new incident data", "incident", msg.incident.ID) + m.incidentCache[msg.incident.ID] = &cachedIncidentData{ + incident: msg.incident, + dataLoaded: true, + lastFetched: time.Now(), + } + } + + // Only update selected incident if no incident is selected or this matches the selected one + // Skip if we're viewing a different incident (don't let background pre-fetch overwrite it) + if m.selectedIncident == nil || msg.incident.ID == m.selectedIncident.ID { + m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) + m.selectedIncident = msg.incident + m.incidentDataLoaded = true + + if m.viewingIncident { + return m, func() tea.Msg { return renderIncidentMsg("refresh") } + } + } case gotIncidentNotesMsg: if msg.err != nil { return m, func() tea.Msg { return errMsg{msg.err} } } - // CANNOT refer to the m.SelectedIncident, because it may not have - // completed yet, and will be nil - switch { - case len(msg.notes) == 1: - m.setStatus(fmt.Sprintf("got %d note for incident", len(msg.notes))) - case len(msg.notes) > 1: - m.setStatus(fmt.Sprintf("got %d notes for incident", len(msg.notes))) + // Update cache with fetched notes + if cached, exists := m.incidentCache[msg.incidentID]; exists { + log.Debug("Update", "gotIncidentNotesMsg", "refreshing cached notes", "incident", msg.incidentID, "count", len(msg.notes)) + cached.notes = msg.notes + cached.notesLoaded = true + } else { + log.Debug("Update", "gotIncidentNotesMsg", "caching new notes", "incident", msg.incidentID, "count", len(msg.notes)) + m.incidentCache[msg.incidentID] = &cachedIncidentData{ + notes: msg.notes, + notesLoaded: true, + } } - m.selectedIncidentNotes = msg.notes - if m.viewingIncident { - cmds = append(cmds, renderIncident(&m)) + // Only update selected incident notes if no incident is selected or this matches the selected one + // Skip if we're viewing a different incident (don't let background pre-fetch overwrite it) + if m.selectedIncident == nil || msg.incidentID == m.selectedIncident.ID { + switch { + case len(msg.notes) == 1: + m.setStatus(fmt.Sprintf("got %d note for incident", len(msg.notes))) + case len(msg.notes) > 1: + m.setStatus(fmt.Sprintf("got %d notes for incident", len(msg.notes))) + } + + m.selectedIncidentNotes = msg.notes + m.incidentNotesLoaded = true + + if m.viewingIncident { + cmds = append(cmds, renderIncident(&m)) + } } case gotIncidentAlertsMsg: @@ -185,18 +236,35 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{msg.err} } } - // CANNOT refer to the m.SelectedIncident, because it may not have - // completed yet, and will be nil - switch { - case len(msg.alerts) == 1: - m.setStatus(fmt.Sprintf("got %d alert for incident", len(msg.alerts))) - case len(msg.alerts) > 1: - m.setStatus(fmt.Sprintf("got %d alerts for incident", len(msg.alerts))) + // Update cache with fetched alerts + if cached, exists := m.incidentCache[msg.incidentID]; exists { + log.Debug("Update", "gotIncidentAlertsMsg", "refreshing cached alerts", "incident", msg.incidentID, "count", len(msg.alerts)) + cached.alerts = msg.alerts + cached.alertsLoaded = true + } else { + log.Debug("Update", "gotIncidentAlertsMsg", "caching new alerts", "incident", msg.incidentID, "count", len(msg.alerts)) + m.incidentCache[msg.incidentID] = &cachedIncidentData{ + alerts: msg.alerts, + alertsLoaded: true, + } } - m.selectedIncidentAlerts = msg.alerts - if m.viewingIncident { - cmds = append(cmds, renderIncident(&m)) + // Only update selected incident alerts if no incident is selected or this matches the selected one + // Skip if we're viewing a different incident (don't let background pre-fetch overwrite it) + if m.selectedIncident == nil || msg.incidentID == m.selectedIncident.ID { + switch { + case len(msg.alerts) == 1: + m.setStatus(fmt.Sprintf("got %d alert for incident", len(msg.alerts))) + case len(msg.alerts) > 1: + m.setStatus(fmt.Sprintf("got %d alerts for incident", len(msg.alerts))) + } + + m.selectedIncidentAlerts = msg.alerts + m.incidentAlertsLoaded = true + + if m.viewingIncident { + cmds = append(cmds, renderIncident(&m)) + } } case updateIncidentListMsg: @@ -245,6 +313,19 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Overwrite m.incidentList with current incidents m.incidentList = msg.incidents + // Pre-fetch incident data for all incidents in the list + for _, i := range m.incidentList { + // Check if incident is already cached + if _, exists := m.incidentCache[i.ID]; !exists { + // Not cached - pre-fetch all data in the background + cmds = append(cmds, + getIncident(m.config, i.ID), + getIncidentAlerts(m.config, i.ID), + getIncidentNotes(m.config, i.ID), + ) + } + } + // Check if any incidents should be auto-acknowledged; // This must be done before adding the stale incidents for _, i := range m.incidentList { @@ -260,6 +341,19 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Add stale incidents to the list m.incidentList = append(m.incidentList, staleIncidentList...) + // Clean up cache - remove entries for incidents no longer in the list + // (including STALE incidents, but excluding those that have aged out) + incidentIDs := make(map[string]bool) + for _, i := range m.incidentList { + incidentIDs[i.ID] = true + } + for id := range m.incidentCache { + if !incidentIDs[id] { + delete(m.incidentCache, id) + log.Debug("Update", "updatedIncidentListMsg", "removing cached data for incident no longer in list", "incident", id) + } + } + var totalIncidentCount int var rows []table.Row @@ -366,15 +460,15 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{fmt.Errorf("unsupported OS: no browser open command available")} } } c := []string{defaultBrowserOpenCommand} - return m, func() tea.Msg { return openBrowserCmd(c, m.selectedIncident.HTMLURL) } + return m, openBrowserCmd(c, m.selectedIncident.HTMLURL) case browserFinishedMsg: if msg.err != nil { + m.setStatus(fmt.Sprintf("failed to open browser: %s", msg.err)) return m, func() tea.Msg { return errMsg{msg.err} } - } else { - m.setStatus(fmt.Sprintf("opened incident %s in browser - check browser window", m.selectedIncident.ID)) - return m, nil } + m.setStatus(fmt.Sprintf("opened incident %s in browser - check browser window", m.selectedIncident.ID)) + return m, nil // This is a catch all for any action that requires a selected incident case waitForSelectedIncidentThenDoMsg: @@ -400,6 +494,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case renderIncidentMsg: if m.selectedIncident == nil { m.setStatus("failed render incidents - no incidents provided") + m.viewingIncident = false return m, nil } @@ -436,28 +531,47 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case acknowledgedIncidentsMsg: - var incidentIDs []string if msg.err != nil { return m, func() tea.Msg { return errMsg{msg.err} } } - for _, i := range msg.incidents { - incidentIDs = append(incidentIDs, i.ID) - } - incidents := strings.Join(incidentIDs, " ") - m.setStatus(fmt.Sprintf("acknowledged incidents: %s", incidents)) + incidentIDs := strings.Join(getIDsFromIncidents(msg.incidents), " ") + m.setStatus(fmt.Sprintf("acknowledged incidents: %s", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: acknowledgedIncidentsMsg") } case unAcknowledgedIncidentsMsg: - var incidentIDs []string if msg.err != nil { return m, func() tea.Msg { return errMsg{msg.err} } } - for _, i := range msg.incidents { - incidentIDs = append(incidentIDs, i.ID) + incidentIDs := strings.Join(getIDsFromIncidents(msg.incidents), " ") + m.setStatus(fmt.Sprintf("un-acknowledged incidents: %s", incidentIDs)) + + // After un-acknowledging, re-escalate each incident to its escalation policy to page current on-call + // Group incidents by escalation policy + policyGroups := make(map[string][]pagerduty.Incident) + for _, incident := range msg.incidents { + policyKey := getEscalationPolicyKey(incident.Service.ID, m.config.EscalationPolicies) + policyGroups[policyKey] = append(policyGroups[policyKey], incident) + } + + // Create re-escalate messages for each policy group + var cmds []tea.Cmd + for policyKey, incidents := range policyGroups { + policy := m.config.EscalationPolicies[policyKey] + if policy != nil && policy.ID != "" { + cmds = append(cmds, func() tea.Msg { + return reEscalateIncidentsMsg{ + incidents: incidents, + policy: policy, + level: reEscalateDefaultPolicyLevel, + } + }) + } + } + + if len(cmds) > 0 { + return m, tea.Batch(cmds...) } - incidents := strings.Join(incidentIDs, " ") - m.setStatus(fmt.Sprintf("re-escalated incidents: %s", incidents)) return m, func() tea.Msg { return updateIncidentListMsg("sender: unAcknowledgedIncidentsMsg") } @@ -473,7 +587,8 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case reassignedIncidentsMsg: - m.setStatus(fmt.Sprintf("reassigned incidents %v; refreshing Incident List ", msg)) + incidentIDs := getIDsFromIncidents(msg) + m.setStatus(fmt.Sprintf("reassigned incidents %v; refreshing Incident List ", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: reassignedIncidentsMsg") } case reEscalateIncidentsMsg: @@ -488,7 +603,8 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case reEscalatedIncidentsMsg: - m.setStatus(fmt.Sprintf("re-escalated incidents %v; refreshing Incident List ", msg)) + incidentIDs := getIDsFromIncidents(msg) + m.setStatus(fmt.Sprintf("re-escalated incidents %v; refreshing Incident List ", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: reEscalatedIncidentsMsg") } case silenceSelectedIncidentMsg: diff --git a/pkg/tui/views.go b/pkg/tui/views.go index 92387b9..434c9f2 100644 --- a/pkg/tui/views.go +++ b/pkg/tui/views.go @@ -145,12 +145,9 @@ func (m model) View() string { return errorStyle.Render(s.String()) case m.viewingIncident: - log.Debug("viewingIncident") - - s.WriteString(m.incidentViewer.View()) + s.WriteString(tableContainerStyle.Render(m.incidentViewer.View())) default: - s.WriteString(tableContainerStyle.Render(m.table.View())) } @@ -239,7 +236,30 @@ func (m model) template() (string, error) { } o := new(bytes.Buffer) - summary := summarize(m.selectedIncident, m.selectedIncidentAlerts, m.selectedIncidentNotes) + + // Progressive rendering: show what we have, mark missing parts as loading + var alerts []pagerduty.IncidentAlert + var notes []pagerduty.IncidentNote + + if m.incidentAlertsLoaded { + alerts = m.selectedIncidentAlerts + } else { + // Alerts not loaded yet - will show "Loading alerts..." in template + alerts = nil + } + + if m.incidentNotesLoaded { + notes = m.selectedIncidentNotes + } else { + // Notes not loaded yet - will show "Loading notes..." in template + notes = nil + } + + summary := summarize(m.selectedIncident, alerts, notes) + // Add loading state info to summary for template to use + summary.AlertsLoading = !m.incidentAlertsLoaded + summary.NotesLoading = !m.incidentNotesLoaded + err = template.Execute(o, summary) if err != nil { // TODO: Figure out how to handle this with a proper errMsg @@ -359,6 +379,9 @@ type incidentSummary struct { Alerts []alertSummary Notes []noteSummary Clusters []string + // Progressive rendering flags + AlertsLoading bool + NotesLoading bool } func summarizeIncident(i *pagerduty.Incident) incidentSummary { @@ -433,7 +456,9 @@ Acknowledged by: {{ range $i, $ack := .Acknowledged -}} ## Notes -{{ if .Notes }} +{{ if .NotesLoading }} +_Loading notes..._ +{{ else if .Notes }} {{ range $note := .Notes }} > {{ $note.Content }} -- {{ $note.User }} @ {{ $note.Created }} @@ -442,8 +467,11 @@ Acknowledged by: {{ range $i, $ack := .Acknowledged -}} _none_ {{ end }} -## Alerts ({{ len .Alerts }}) +## Alerts{{ if not .AlertsLoading }} ({{ len .Alerts }}){{ end }} +{{ if .AlertsLoading }} +_Loading alerts..._ +{{ else }} {{ range $alert := .Alerts }} {{ $alert.ID }} - {{ $alert.Status }} {{ if $alert.Name }} @@ -459,9 +487,10 @@ _{{- $alert.Name }}_ Details : {{ range $key, $value := $alert.Details }} -* {{ $key }}: {{ $value }} +* {{ $key }}: {{ $value }} {{ end }} +{{ end }} {{ end }} `