diff --git a/docs/astro.config.mjs b/docs/astro.config.mjs index 05dcfdfec..1b12ebbf7 100644 --- a/docs/astro.config.mjs +++ b/docs/astro.config.mjs @@ -74,6 +74,7 @@ export default defineConfig({ "configuration/searching", "configuration/pr-section", "configuration/issue-section", + "configuration/notification-section", "configuration/repo-paths", "configuration/keybindings", "configuration/theme", diff --git a/docs/src/content/docs/configuration/defaults.mdx b/docs/src/content/docs/configuration/defaults.mdx index d2628148a..a4be10750 100644 --- a/docs/src/content/docs/configuration/defaults.mdx +++ b/docs/src/content/docs/configuration/defaults.mdx @@ -10,6 +10,7 @@ settings on a per-section basis. ```yaml defaults: issuesLimit: 20 + notificationsLimit: 20 prApproveComment: LGTM preview: open: true @@ -22,7 +23,7 @@ defaults: By default, the dashboard is configured to: - Display the preview pane with a width of 50 columns for all work items. -- Only fetch 20 PRs and issues at a time for each section. +- Only fetch 20 PRs, issues, and notifications at a time for each section. - Display the PRs view when the dashboard loads. - Refetch PRs and issues for each section every 30 minutes. - Display dates using relative values. @@ -110,6 +111,21 @@ This setting defines how many issues the dashboard should fetch for each section [refresh current section]: /getting-started/keybindings/global/#r---refresh-current-section [refresh all sections]: /getting-started/keybindings/global/#r---refresh-all-sections +### Notifications Fetch Limit (`notificationsLimit`) + +| Type | Minimum | Default | +| :------ | :-----: | :-----: | +| Integer | 1 | 20 | + +This setting defines how many notifications the dashboard should fetch when: + +- The dashboard first loads. +- You navigate to the next notification in a table without another fetched notification to display. +- You use the [refresh current section] or [refresh all sections] commands. + +[refresh current section]: /getting-started/keybindings/global/#r---refresh-current-section +[refresh all sections]: /getting-started/keybindings/global/#r---refresh-all-sections + ### Preview Pane (`preview`) These settings define how the preview pane displays in the dashboard. You can specify diff --git a/docs/src/content/docs/configuration/examples.mdx b/docs/src/content/docs/configuration/examples.mdx index 12f7dd4f3..29bf6b89f 100644 --- a/docs/src/content/docs/configuration/examples.mdx +++ b/docs/src/content/docs/configuration/examples.mdx @@ -31,6 +31,24 @@ issuesSections: - title: "Involved" filters: "is:open involves:@me -author:@me" +notificationsSections: + - title: "All" + filters: "" + - title: "Created" + filters: "reason:author" + - title: "Participating" + filters: "reason:participating" + - title: "Mentioned" + filters: "reason:mention" + - title: "Review Requested" + filters: "reason:review-requested" + - title: "Assigned" + filters: "reason:assign" + - title: "Subscribed" + filters: "reason:subscribed" + - title: "Team Mentioned" + filters: "reason:team-mention" + pager: diff: less showAuthorIcons: true @@ -52,6 +70,7 @@ defaults: width: 84 prsLimit: 20 issuesLimit: 20 + notificationsLimit: 20 theme: ui: diff --git a/docs/src/content/docs/configuration/layout/options.mdx b/docs/src/content/docs/configuration/layout/options.mdx index 88e65dab8..a9e7a4d77 100644 --- a/docs/src/content/docs/configuration/layout/options.mdx +++ b/docs/src/content/docs/configuration/layout/options.mdx @@ -4,8 +4,9 @@ title: Valid Options ## Valid Layout Options -Any column can define the [`grow`], [`width`], and [`hidden`] options. +Any column can define the [`align`], [`grow`], [`width`], and [`hidden`] options. +[`align`]: #column-alignment [`hidden`]: #hide-column [`grow`]: #grow-column [`width`]: #column-width @@ -42,4 +43,26 @@ font to be monospace, this is a reliable way to ensure a minimum width for reada | `hidden` | boolean | false | Specify whether the column should be hidden from view. Set this value to `true` to hide the -column or `true` to show it. +column or `false` to show it. + +## Column Alignment + +| Property | Type | Default | +| :------- | :----- | :------ | +| `align` | string | `left` | + +Specify the horizontal alignment of the column's content. Valid values are: + +- `left` — Align content to the left (default) +- `right` — Align content to the right +- `center` — Center the content + +This is useful for numeric columns like comment counts where right-alignment improves readability. + +```yaml +prs: + layout: + numComments: + width: 5 + align: right +``` diff --git a/docs/src/content/docs/configuration/notification-section.mdx b/docs/src/content/docs/configuration/notification-section.mdx new file mode 100644 index 000000000..a5a73cfff --- /dev/null +++ b/docs/src/content/docs/configuration/notification-section.mdx @@ -0,0 +1,135 @@ +--- +title: Notification Sections +--- + +# Notification Section Options (`notificationsSections`) + +Defines sections in the dashboard's Notifications view. + +- Every section must define a [`title`] and [`filters`]. +- When you define [`limit`] for a section, that value overrides the + [`defaults.notificationsLimit`] setting. + +[`title`]: #notification-title-title +[`filters`]: #notification-filters-filters +[`limit`]: #notification-fetch-limit-limit +[`defaults.notificationsLimit`]: /configuration/defaults/#notifications-fetch-limit-notificationslimit + +## Search Section + +The Notifications view includes a search section (indicated by a magnifying glass icon) as the first tab. This serves as a scratchpad for one-off searches without modifying your configured sections. + +- Default filter: `archived:false` +- Respects `smartFilteringAtLaunch`: when enabled and running from a git repository, the search automatically scopes to that repo +- Use the `/` key to focus the search bar and enter custom queries + +## Default Sections + +By default, the dashboard includes these notification sections: + +```yaml +notificationsSections: + - title: All + filters: "" + - title: Created + filters: "reason:author" + - title: Participating + filters: "reason:participating" + - title: Mentioned + filters: "reason:mention" + - title: Review Requested + filters: "reason:review-requested" + - title: Assigned + filters: "reason:assign" + - title: Subscribed + filters: "reason:subscribed" + - title: Team Mentioned + filters: "reason:team-mention" +``` + +You can customize these by defining your own `notificationsSections` in your config file. + +## Notification Title (`title`) + +This setting defines the section's name. The dashboard displays this value in the tabs for +the Notifications view. + +## Notification Filters (`filters`) + +This setting defines the filters for notifications in the section's table. Notification filters differ from PR and Issue filters because they use client-side filtering. + +### Available Filters + +#### Read State Filters + +| Filter | Description | +|--------|-------------| +| `is:unread` | Show only unread notifications | +| `is:read` | Show only read notifications | +| `is:all` | Show both read and unread notifications | +| `is:done` | Show archived/done notifications | + +#### Reason Filters + +| Filter | Description | +|--------|-------------| +| `reason:author` | Activity on threads you created | +| `reason:comment` | Someone commented on a thread you're subscribed to | +| `reason:mention` | You were @mentioned | +| `reason:review-requested` | Your review was requested on a PR | +| `reason:assign` | You were assigned | +| `reason:subscribed` | Activity on threads you're watching | +| `reason:team-mention` | Your team was @mentioned | +| `reason:state-change` | Thread state changed (merged, closed, etc.) | +| `reason:ci-activity` | CI workflow activity | +| `reason:participating` | Meta-filter that expands to: author, comment, mention, review-requested, assign, state-change | + +#### Repository Filters + +| Filter | Description | +|--------|-------------| +| `repo:owner/name` | Show notifications only from the specified repository | + +### Filter Examples + +```yaml +# Show only notifications where you were mentioned +- title: Mentioned + filters: "reason:mention" + +# Show unread notifications from a specific repo +- title: My Project + filters: "is:unread repo:myorg/myproject" + +# Show notifications where you're actively participating +- title: Participating + filters: "reason:participating" + +# Combine multiple reason filters +- title: Review & Mentions + filters: "reason:review-requested reason:mention" +``` + +### Filter Behavior + +- **Default behavior**: With no filters or empty filters, the section shows unread notifications plus any bookmarked notifications +- **Explicit `is:unread`**: Shows only unread notifications, excluding bookmarked read notifications +- **Reason filters**: Applied client-side after fetching from GitHub's API + +## Notification Fetch Limit (`limit`) + +| Type | Minimum | Default | +| :------ | :-----: | :-----: | +| Integer | 1 | 20 | + +This setting defines how many notifications the dashboard should fetch for the section when: + +- The dashboard first loads. +- You navigate to the next notification in a table without another fetched notification to display. +- You use the [refresh current section] or [refresh all sections] commands. + +This setting overrides the [`defaults.notificationsLimit`] setting. + +[refresh current section]: /getting-started/keybindings/global/#r---refresh-current-section +[refresh all sections]: /getting-started/keybindings/global/#r---refresh-all-sections +[`defaults.notificationsLimit`]: /configuration/defaults/#notifications-fetch-limit-notificationslimit diff --git a/docs/src/content/docs/configuration/theme.mdx b/docs/src/content/docs/configuration/theme.mdx index e71c51d4f..4d5df41a1 100644 --- a/docs/src/content/docs/configuration/theme.mdx +++ b/docs/src/content/docs/configuration/theme.mdx @@ -41,6 +41,7 @@ colors: faint: "#8a8a8a" warning: "#800000" success: "#008000" + actor: "#c6c6c6" background: selected: "#808080" border: @@ -212,6 +213,18 @@ dashboard UI: - The icon for passing checks for PRs in the preview pane - Success messages for commands, like when the dashboard fetches work items. +#### Actor Text Color + +| Property | Type | default | +| :------- | :--- | :---------------------------------------- | +| `actor` | hex | `#c6c6c6` for dark or `#808080` for light | + +This setting determines the color of the text for the following elements in the +dashboard UI: + +- The username of the person who triggered a notification, displayed after the + notification title in the notifications view + ### Background Colors (`background`) Defines the background colors for the dashboard. By default, the background color for diff --git a/internal/config/parser.go b/internal/config/parser.go index 76492ca36..2d79c6413 100644 --- a/internal/config/parser.go +++ b/internal/config/parser.go @@ -57,6 +57,8 @@ func (a *ViewType) UnmarshalJSON(b []byte) error { return err } switch strings.ToLower(s) { + case "notifications": + *a = NotificationsView case "prs": *a = PRsView case "issues": @@ -69,9 +71,10 @@ func (a *ViewType) UnmarshalJSON(b []byte) error { } const ( - PRsView ViewType = "prs" - IssuesView ViewType = "issues" - RepoView ViewType = "repo" + NotificationsView ViewType = "notifications" + PRsView ViewType = "prs" + IssuesView ViewType = "issues" + RepoView ViewType = "repo" ) type SectionConfig struct { @@ -96,6 +99,12 @@ type IssuesSectionConfig struct { Layout IssuesLayoutConfig `yaml:"layout,omitempty"` } +type NotificationsSectionConfig struct { + Title string + Filters string + Limit *int `yaml:"limit,omitempty"` +} + type PreviewConfig struct { Open bool Width int @@ -174,6 +183,7 @@ type Defaults struct { PrsLimit int `yaml:"prsLimit"` PrApproveComment string `yaml:"prApproveComment,omitempty"` IssuesLimit int `yaml:"issuesLimit"` + NotificationsLimit int `yaml:"notificationsLimit"` View ViewType `yaml:"view"` Layout LayoutConfig `yaml:"layout,omitempty"` RefetchIntervalMinutes int `yaml:"refetchIntervalMinutes,omitempty"` @@ -246,6 +256,7 @@ type ColorThemeText struct { Warning HexColor `yaml:"warning" validate:"omitempty,hexcolor"` Success HexColor `yaml:"success" validate:"omitempty,hexcolor"` Error HexColor `yaml:"error" validate:"omitempty,hexcolor"` + Actor HexColor `yaml:"actor" validate:"omitempty,hexcolor"` } type ColorThemeBorder struct { @@ -299,17 +310,18 @@ type ThemeConfig struct { } type Config struct { - PRSections []PrsSectionConfig `yaml:"prSections"` - IssuesSections []IssuesSectionConfig `yaml:"issuesSections"` - Repo RepoConfig `yaml:"repo,omitempty"` - Defaults Defaults `yaml:"defaults"` - Keybindings Keybindings `yaml:"keybindings"` - RepoPaths map[string]string `yaml:"repoPaths"` - Theme *ThemeConfig `yaml:"theme,omitempty" validate:"omitempty"` - Pager Pager `yaml:"pager"` - ConfirmQuit bool `yaml:"confirmQuit"` - ShowAuthorIcons bool `yaml:"showAuthorIcons,omitempty"` - SmartFilteringAtLaunch bool `yaml:"smartFilteringAtLaunch" default:"true"` + PRSections []PrsSectionConfig `yaml:"prSections"` + IssuesSections []IssuesSectionConfig `yaml:"issuesSections"` + NotificationsSections []NotificationsSectionConfig `yaml:"notificationsSections"` + Repo RepoConfig `yaml:"repo,omitempty"` + Defaults Defaults `yaml:"defaults"` + Keybindings Keybindings `yaml:"keybindings"` + RepoPaths map[string]string `yaml:"repoPaths"` + Theme *ThemeConfig `yaml:"theme,omitempty" validate:"omitempty"` + Pager Pager `yaml:"pager"` + ConfirmQuit bool `yaml:"confirmQuit"` + ShowAuthorIcons bool `yaml:"showAuthorIcons,omitempty"` + SmartFilteringAtLaunch bool `yaml:"smartFilteringAtLaunch" default:"true"` } type configError struct { @@ -332,6 +344,7 @@ func (parser ConfigParser) getDefaultConfig() Config { PrsLimit: 20, PrApproveComment: "LGTM", IssuesLimit: 20, + NotificationsLimit: 20, View: PRsView, RefetchIntervalMinutes: 30, Layout: LayoutConfig{ @@ -418,6 +431,40 @@ func (parser ConfigParser) getDefaultConfig() Config { Filters: "is:open involves:@me -author:@me", }, }, + NotificationsSections: []NotificationsSectionConfig{ + { + Title: "All", + Filters: "", + }, + { + Title: "Created", + Filters: "reason:author", + }, + { + Title: "Participating", + Filters: "reason:participating", + }, + { + Title: "Mentioned", + Filters: "reason:mention", + }, + { + Title: "Review Requested", + Filters: "reason:review-requested", + }, + { + Title: "Assigned", + Filters: "reason:assign", + }, + { + Title: "Subscribed", + Filters: "reason:subscribed", + }, + { + Title: "Team Mentioned", + Filters: "reason:team-mention", + }, + }, Keybindings: Keybindings{ Universal: []Keybinding{}, Issues: []Keybinding{}, @@ -588,6 +635,7 @@ func (parser ConfigParser) mergeConfigs(globalCfgPath, userProvidedCfgPath strin dest["keybindings"].(map[string]any)["issues"] = issuesKeybinds dest["prSections"] = overridesCopy["prSections"] dest["issuesSections"] = overridesCopy["issuesSections"] + dest["notificationsSections"] = overridesCopy["notificationsSections"] return nil })); err != nil { diff --git a/internal/config/testdata/global-config.golden.yml b/internal/config/testdata/global-config.golden.yml index b328dba3f..703aa5917 100644 --- a/internal/config/testdata/global-config.golden.yml +++ b/internal/config/testdata/global-config.golden.yml @@ -19,6 +19,23 @@ issuesSections: filters: author:@me repo:dlvhdr/gh-dash is:open - title: All filters: repo:dlvhdr/gh-dash sort:reactions +notificationsSections: + - title: All + filters: "" + - title: Created + filters: "reason:author" + - title: Participating + filters: "reason:participating" + - title: Mentioned + filters: "reason:mention" + - title: Review Requested + filters: "reason:review-requested" + - title: Assigned + filters: "reason:assign" + - title: Subscribed + filters: "reason:subscribed" + - title: Team Mentioned + filters: "reason:team-mention" repo: branchesRefetchIntervalSeconds: 30 prsRefetchIntervalSeconds: 60 @@ -29,6 +46,7 @@ defaults: prsLimit: 5 prApproveComment: LGTM issuesLimit: 5 + notificationsLimit: 20 view: prs layout: prs: diff --git a/internal/config/testdata/merged-config.golden.yml b/internal/config/testdata/merged-config.golden.yml index 89f94c27e..c222521e8 100644 --- a/internal/config/testdata/merged-config.golden.yml +++ b/internal/config/testdata/merged-config.golden.yml @@ -9,6 +9,23 @@ prSections: issuesSections: - title: Open filters: author:@me -author:@me sort:reactions +notificationsSections: + - title: All + filters: "" + - title: Created + filters: "reason:author" + - title: Participating + filters: "reason:participating" + - title: Mentioned + filters: "reason:mention" + - title: Review Requested + filters: "reason:review-requested" + - title: Assigned + filters: "reason:assign" + - title: Subscribed + filters: "reason:subscribed" + - title: Team Mentioned + filters: "reason:team-mention" repo: branchesRefetchIntervalSeconds: 30 prsRefetchIntervalSeconds: 60 @@ -19,6 +36,7 @@ defaults: width: 80 prsLimit: 100 issuesLimit: 100 + notificationsLimit: 100 view: prs layout: prs: diff --git a/internal/config/testdata/other-test-config.yml b/internal/config/testdata/other-test-config.yml index c1e475932..e2825a015 100644 --- a/internal/config/testdata/other-test-config.yml +++ b/internal/config/testdata/other-test-config.yml @@ -26,6 +26,7 @@ defaults: hidden: true prsLimit: 100 issuesLimit: 100 + notificationsLimit: 100 theme: colors: diff --git a/internal/config/utils.go b/internal/config/utils.go index b9aaf6fc8..bfccc9621 100644 --- a/internal/config/utils.go +++ b/internal/config/utils.go @@ -45,6 +45,14 @@ func (cfg IssuesSectionConfig) ToSectionConfig() SectionConfig { } } +func (cfg NotificationsSectionConfig) ToSectionConfig() SectionConfig { + return SectionConfig{ + Title: cfg.Title, + Filters: cfg.Filters, + Limit: cfg.Limit, + } +} + func MergeColumnConfigs(defaultCfg, sectionCfg ColumnConfig) ColumnConfig { colCfg := defaultCfg if sectionCfg.Width != nil { diff --git a/internal/data/bookmarks.go b/internal/data/bookmarks.go new file mode 100644 index 000000000..87f950c9d --- /dev/null +++ b/internal/data/bookmarks.go @@ -0,0 +1,235 @@ +package data + +import ( + "encoding/json" + "os" + "path/filepath" + "sync" + + "github.com/charmbracelet/log" +) + +const ( + defaultXDGStateDir = ".local/state" + dashDir = "gh-dash" +) + +// NotificationIDStore is a generic store for persisting sets of notification IDs. +// Used for bookmarks, done notifications, and similar features. +type NotificationIDStore struct { + mu sync.RWMutex + ids map[string]bool + filePath string + name string // for logging +} + +func newNotificationIDStore(filename, name string) *NotificationIDStore { + store := &NotificationIDStore{ + ids: make(map[string]bool), + name: name, + } + filePath, err := getStateFilePath(filename) + if err != nil { + log.Error("Failed to get state file path for "+name, "err", err) + } + store.filePath = filePath + if err := store.load(); err != nil { + log.Error("Failed to load "+name, "err", err) + } + return store +} + +func getStateFilePath(filename string) (string, error) { + stateDir := os.Getenv("XDG_STATE_HOME") + if stateDir == "" { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", err + } + stateDir = filepath.Join(homeDir, defaultXDGStateDir) + } + return filepath.Join(stateDir, dashDir, filename), nil +} + +func (s *NotificationIDStore) load() error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.filePath == "" { + return nil + } + + data, err := os.ReadFile(s.filePath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + + var idList []string + if err := json.Unmarshal(data, &idList); err != nil { + return err + } + + for _, id := range idList { + s.ids[id] = true + } + log.Debug("Loaded "+s.name, "count", len(s.ids)) + return nil +} + +func (s *NotificationIDStore) save() error { + s.mu.RLock() + defer s.mu.RUnlock() + + if s.filePath == "" { + return nil + } + + idList := make([]string, 0, len(s.ids)) + for id := range s.ids { + idList = append(idList, id) + } + + data, err := json.MarshalIndent(idList, "", " ") + if err != nil { + return err + } + + dir := filepath.Dir(s.filePath) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + + // Use atomic write: write to temp file, then rename. + // This prevents races when multiple async saves run concurrently. + tmpFile, err := os.CreateTemp(dir, ".tmp-*") + if err != nil { + return err + } + tmpPath := tmpFile.Name() + if _, err := tmpFile.Write(data); err != nil { + tmpFile.Close() + os.Remove(tmpPath) + return err + } + if err := tmpFile.Close(); err != nil { + os.Remove(tmpPath) + return err + } + if err := os.Rename(tmpPath, s.filePath); err != nil { + os.Remove(tmpPath) + return err + } + + log.Debug("Saved "+s.name, "count", len(idList)) + return nil +} + +// Has checks if an ID is in the store +func (s *NotificationIDStore) Has(id string) bool { + s.mu.RLock() + defer s.mu.RUnlock() + return s.ids[id] +} + +// Add adds an ID to the store +func (s *NotificationIDStore) Add(id string) { + s.mu.Lock() + s.ids[id] = true + s.mu.Unlock() + go s.save() // Async save to avoid blocking UI +} + +// Remove removes an ID from the store +func (s *NotificationIDStore) Remove(id string) { + s.mu.Lock() + delete(s.ids, id) + s.mu.Unlock() + go s.save() // Async save to avoid blocking UI +} + +// Toggle toggles an ID in the store, returns the new state +func (s *NotificationIDStore) Toggle(id string) bool { + s.mu.Lock() + newState := !s.ids[id] + if newState { + s.ids[id] = true + } else { + delete(s.ids, id) + } + s.mu.Unlock() + go s.save() // Async save to avoid blocking UI + return newState +} + +// GetAll returns all IDs in the store +func (s *NotificationIDStore) GetAll() []string { + s.mu.RLock() + defer s.mu.RUnlock() + ids := make([]string, 0, len(s.ids)) + for id := range s.ids { + ids = append(ids, id) + } + return ids +} + +// Flush forces an immediate synchronous save. Useful for testing. +func (s *NotificationIDStore) Flush() error { + return s.save() +} + +// Singletons for bookmark and done stores + +var ( + bookmarkStore *NotificationIDStore + bookmarkStoreOnce sync.Once + doneStore *NotificationIDStore + doneStoreOnce sync.Once +) + +// GetBookmarkStore returns the singleton bookmark store +func GetBookmarkStore() *NotificationIDStore { + bookmarkStoreOnce.Do(func() { + bookmarkStore = newNotificationIDStore("bookmarks.json", "bookmarks") + }) + return bookmarkStore +} + +// GetDoneStore returns the singleton done store +func GetDoneStore() *NotificationIDStore { + doneStoreOnce.Do(func() { + doneStore = newNotificationIDStore("done.json", "done notifications") + }) + return doneStore +} + +// Convenience methods for BookmarkStore (maintains API compatibility) + +// IsBookmarked checks if a notification is bookmarked +func (s *NotificationIDStore) IsBookmarked(id string) bool { + return s.Has(id) +} + +// ToggleBookmark toggles the bookmark state +func (s *NotificationIDStore) ToggleBookmark(id string) bool { + return s.Toggle(id) +} + +// GetBookmarkedIds returns all bookmarked IDs +func (s *NotificationIDStore) GetBookmarkedIds() []string { + return s.GetAll() +} + +// Convenience methods for DoneStore + +// IsDone checks if a notification is marked as done +func (s *NotificationIDStore) IsDone(id string) bool { + return s.Has(id) +} + +// MarkDone marks a notification as done +func (s *NotificationIDStore) MarkDone(id string) { + s.Add(id) +} diff --git a/internal/data/bookmarks_test.go b/internal/data/bookmarks_test.go new file mode 100644 index 000000000..b30532e96 --- /dev/null +++ b/internal/data/bookmarks_test.go @@ -0,0 +1,326 @@ +package data + +import ( + "os" + "path/filepath" + "testing" +) + +func TestNotificationIDStore(t *testing.T) { + // Create a temp directory for test files + tempDir, err := os.MkdirTemp("", "gh-dash-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + testFile := filepath.Join(tempDir, "test-store.json") + + t.Run("new store is empty", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + if store.Has("id1") { + t.Error("New store should not have any IDs") + } + if len(store.GetAll()) != 0 { + t.Error("New store should return empty list") + } + }) + + t.Run("Add and Has", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + store.Add("id1") + if !store.Has("id1") { + t.Error("Store should have id1 after Add") + } + if store.Has("id2") { + t.Error("Store should not have id2") + } + }) + + t.Run("Remove", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + store.Add("id1") + store.Remove("id1") + if store.Has("id1") { + t.Error("Store should not have id1 after Remove") + } + }) + + t.Run("Toggle", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + // Toggle on + if !store.Toggle("id1") { + t.Error("First toggle should return true") + } + if !store.Has("id1") { + t.Error("Store should have id1 after toggle on") + } + + // Toggle off + if store.Toggle("id1") { + t.Error("Second toggle should return false") + } + if store.Has("id1") { + t.Error("Store should not have id1 after toggle off") + } + }) + + t.Run("GetAll", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + store.Add("id1") + store.Add("id2") + store.Add("id3") + + all := store.GetAll() + if len(all) != 3 { + t.Errorf("GetAll should return 3 IDs, got %d", len(all)) + } + + // Check all IDs are present (order not guaranteed) + found := make(map[string]bool) + for _, id := range all { + found[id] = true + } + for _, id := range []string{"id1", "id2", "id3"} { + if !found[id] { + t.Errorf("GetAll should include %s", id) + } + } + }) + + t.Run("persistence", func(t *testing.T) { + persistFile := filepath.Join(tempDir, "persist-test.json") + + // Create store, add IDs, flush to ensure save completes + store1 := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: persistFile, + name: "test", + } + store1.Add("persist1") + store1.Add("persist2") + if err := store1.Flush(); err != nil { + t.Fatalf("Failed to flush: %v", err) + } + + // Create new store, load from same file + store2 := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: persistFile, + name: "test", + } + if err := store2.load(); err != nil { + t.Fatalf("Failed to load: %v", err) + } + + if !store2.Has("persist1") { + t.Error("Loaded store should have persist1") + } + if !store2.Has("persist2") { + t.Error("Loaded store should have persist2") + } + }) + + t.Run("convenience methods", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + // Bookmark convenience methods + store.Add("bookmark1") + if !store.IsBookmarked("bookmark1") { + t.Error("IsBookmarked should return true") + } + if store.ToggleBookmark("bookmark1") { + t.Error("ToggleBookmark should return false (toggled off)") + } + if len(store.GetBookmarkedIds()) != 0 { + t.Error("GetBookmarkedIds should be empty after toggle off") + } + + // Done convenience methods + store.MarkDone("done1") + if !store.IsDone("done1") { + t.Error("IsDone should return true after MarkDone") + } + }) + + t.Run("load from non-existent file", func(t *testing.T) { + nonExistentFile := filepath.Join(tempDir, "non-existent.json") + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: nonExistentFile, + name: "test", + } + + // load should not error for non-existent file, just return empty store + err := store.load() + if err != nil { + t.Errorf("load() from non-existent file should not error, got: %v", err) + } + if len(store.GetAll()) != 0 { + t.Error("store should be empty after loading from non-existent file") + } + }) + + t.Run("load from empty file", func(t *testing.T) { + emptyFile := filepath.Join(tempDir, "empty.json") + if err := os.WriteFile(emptyFile, []byte(""), 0o644); err != nil { + t.Fatalf("Failed to create empty file: %v", err) + } + + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: emptyFile, + name: "test", + } + + err := store.load() + // Empty file should either be handled gracefully or return an error + // The important thing is it doesn't panic + if err != nil { + // This is acceptable - empty file is not valid JSON + t.Logf("load() from empty file returned error (expected): %v", err) + } + }) + + t.Run("load from corrupted JSON", func(t *testing.T) { + corruptedFile := filepath.Join(tempDir, "corrupted.json") + if err := os.WriteFile(corruptedFile, []byte("{invalid json"), 0o644); err != nil { + t.Fatalf("Failed to create corrupted file: %v", err) + } + + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: corruptedFile, + name: "test", + } + + err := store.load() + if err == nil { + t.Error("load() from corrupted JSON should return error") + } + }) + + t.Run("Add same ID multiple times", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + store.Add("id1") + store.Add("id1") + store.Add("id1") + + // Should still only have one ID + all := store.GetAll() + if len(all) != 1 { + t.Errorf("Adding same ID multiple times should result in 1 entry, got %d", len(all)) + } + }) + + t.Run("Remove non-existent ID", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + // Should not panic or error + store.Remove("non-existent") + if store.Has("non-existent") { + t.Error("Has should return false for non-existent ID") + } + }) + + t.Run("empty string ID", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: testFile, + name: "test", + } + + store.Add("") + // Empty string should be handled (though it's an edge case) + if !store.Has("") { + t.Error("Store should have empty string ID after Add") + } + }) + + t.Run("special characters in ID", func(t *testing.T) { + store := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: filepath.Join(tempDir, "special-chars.json"), + name: "test", + } + + specialIds := []string{ + "id-with-dash", + "id_with_underscore", + "id.with.dots", + "id:with:colons", + "id/with/slashes", + "12345678901234567890", // long numeric ID + } + + for _, id := range specialIds { + store.Add(id) + } + + // Verify all were added + for _, id := range specialIds { + if !store.Has(id) { + t.Errorf("Store should have ID %q", id) + } + } + + // Flush to ensure async save completes before testing persistence + if err := store.Flush(); err != nil { + t.Fatalf("Failed to flush: %v", err) + } + + // Test persistence with special characters + store2 := &NotificationIDStore{ + ids: make(map[string]bool), + filePath: filepath.Join(tempDir, "special-chars.json"), + name: "test", + } + if err := store2.load(); err != nil { + t.Fatalf("Failed to load: %v", err) + } + + for _, id := range specialIds { + if !store2.Has(id) { + t.Errorf("Loaded store should have ID %q", id) + } + } + }) +} diff --git a/internal/data/issueapi.go b/internal/data/issueapi.go index cc7690c7a..7404239cd 100644 --- a/internal/data/issueapi.go +++ b/internal/data/issueapi.go @@ -2,11 +2,13 @@ package data import ( "fmt" + "net/url" "time" "github.com/charmbracelet/log" gh "github.com/cli/go-gh/v2/pkg/api" graphql "github.com/cli/shurcooL-graphql" + "github.com/shurcooL/githubv4" "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) @@ -25,7 +27,7 @@ type IssueData struct { Url string Repository Repository Assignees Assignees `graphql:"assignees(first: 3)"` - Comments IssueComments `graphql:"comments(first: 15)"` + Comments IssueComments `graphql:"comments(last: 15)"` Reactions IssueReactions `graphql:"reactions(first: 1)"` Labels IssueLabels `graphql:"labels(first: 20)"` } @@ -147,3 +149,35 @@ type IssuesResponse struct { TotalCount int PageInfo PageInfo } + +// FetchIssue fetches a single issue by its GitHub URL +func FetchIssue(issueUrl string) (IssueData, error) { + var err error + if cachedClient == nil { + cachedClient, err = gh.NewGraphQLClient(gh.ClientOptions{EnableCache: true, CacheTTL: 5 * time.Minute}) + if err != nil { + return IssueData{}, err + } + } + + var queryResult struct { + Resource struct { + Issue IssueData `graphql:"... on Issue"` + } `graphql:"resource(url: $url)"` + } + parsedUrl, err := url.Parse(issueUrl) + if err != nil { + return IssueData{}, err + } + variables := map[string]any{ + "url": githubv4.URI{URL: parsedUrl}, + } + log.Debug("Fetching Issue", "url", issueUrl) + err = cachedClient.Query("FetchIssue", &queryResult, variables) + if err != nil { + return IssueData{}, err + } + log.Info("Successfully fetched Issue", "url", issueUrl) + + return queryResult.Resource.Issue, nil +} diff --git a/internal/data/notificationapi.go b/internal/data/notificationapi.go new file mode 100644 index 000000000..4f4963b2e --- /dev/null +++ b/internal/data/notificationapi.go @@ -0,0 +1,421 @@ +package data + +import ( + "fmt" + "time" + + "github.com/charmbracelet/log" + gh "github.com/cli/go-gh/v2/pkg/api" +) + +// Notification subject types from GitHub API +const ( + SubjectTypePullRequest = "PullRequest" + SubjectTypeIssue = "Issue" + SubjectTypeDiscussion = "Discussion" + SubjectTypeRelease = "Release" + SubjectTypeCommit = "Commit" + SubjectTypeCheckSuite = "CheckSuite" +) + +// Notification reasons from GitHub API +const ( + ReasonSubscribed = "subscribed" + ReasonReviewRequested = "review_requested" + ReasonMention = "mention" + ReasonAuthor = "author" + ReasonComment = "comment" + ReasonAssign = "assign" + ReasonStateChange = "state_change" + ReasonCIActivity = "ci_activity" + ReasonTeamMention = "team_mention" + ReasonSecurityAlert = "security_alert" +) + +var restClient *gh.RESTClient + +type NotificationSubject struct { + Title string `json:"title"` + Url string `json:"url"` + LatestCommentUrl string `json:"latest_comment_url"` + Type string `json:"type"` // PullRequest, Issue, Discussion, Release, etc. +} + +type NotificationRepository struct { + Id int `json:"id"` + Name string `json:"name"` + FullName string `json:"full_name"` + Private bool `json:"private"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + HtmlUrl string `json:"html_url"` +} + +type NotificationData struct { + Id string `json:"id"` + Unread bool `json:"unread"` + Reason string `json:"reason"` // subscribed, review_requested, mention, etc. + UpdatedAt time.Time `json:"updated_at"` + LastReadAt *time.Time `json:"last_read_at"` + Subject NotificationSubject `json:"subject"` + Repository NotificationRepository `json:"repository"` + Url string `json:"url"` + Subscription string `json:"subscription_url"` +} + +func (n NotificationData) GetTitle() string { + return n.Subject.Title +} + +func (n NotificationData) GetRepoNameWithOwner() string { + return n.Repository.FullName +} + +func (n NotificationData) GetNumber() int { + // Notifications don't have a number, return 0 + return 0 +} + +func (n NotificationData) GetUrl() string { + // Convert API URL to HTML URL + // API URL: https://api.github.com/repos/owner/repo/pulls/123 + // HTML URL: https://github.com/owner/repo/pull/123 + return fmt.Sprintf("https://github.com/%s", n.Repository.FullName) +} + +func (n NotificationData) GetUpdatedAt() time.Time { + return n.UpdatedAt +} + +func (n NotificationData) GetCreatedAt() time.Time { + // Notifications don't have a created_at, use updated_at + return n.UpdatedAt +} + +type NotificationsResponse struct { + Notifications []NotificationData + TotalCount int + PageInfo PageInfo +} + +func getRESTClient() (*gh.RESTClient, error) { + if restClient != nil { + return restClient, nil + } + var err error + restClient, err = gh.DefaultRESTClient() + return restClient, err +} + +// NotificationReadState represents the read state filter for notifications +type NotificationReadState string + +const ( + NotificationStateUnread NotificationReadState = "unread" // Only unread (default) + NotificationStateRead NotificationReadState = "read" // Only read + NotificationStateAll NotificationReadState = "all" // Both read and unread +) + +func FetchNotifications(limit int, repoFilters []string, readState NotificationReadState, pageInfo *PageInfo) (NotificationsResponse, error) { + client, err := getRESTClient() + if err != nil { + return NotificationsResponse{}, err + } + + var allNotifications []NotificationData + + // Build query params + // all=true returns both read and unread notifications + includeAll := readState == NotificationStateRead || readState == NotificationStateAll + allParam := "" + if includeAll { + allParam = "&all=true" + } + + // Determine page number from PageInfo (EndCursor stores the current page as string) + page := 1 + if pageInfo != nil && pageInfo.EndCursor != "" { + fmt.Sscanf(pageInfo.EndCursor, "%d", &page) + } + + if len(repoFilters) == 0 { + // No repo filter, fetch all notifications + path := fmt.Sprintf("notifications?per_page=%d&page=%d%s", limit, page, allParam) + log.Debug("Fetching notifications", "limit", limit, "page", page, "readState", readState) + err = client.Get(path, &allNotifications) + if err != nil { + return NotificationsResponse{}, err + } + } else { + // Fetch notifications for each repo and combine + for _, repo := range repoFilters { + var repoNotifications []NotificationData + path := fmt.Sprintf("repos/%s/notifications?per_page=%d&page=%d%s", repo, limit, page, allParam) + log.Debug("Fetching notifications for repo", "repo", repo, "limit", limit, "page", page, "readState", readState) + err = client.Get(path, &repoNotifications) + if err != nil { + log.Warn("Failed to fetch notifications for repo", "repo", repo, "err", err) + continue + } + allNotifications = append(allNotifications, repoNotifications...) + } + } + + // Determine if there's a next page BEFORE filtering (based on raw API response count). + // If the API returned a full page, there are likely more notifications on the server. + // We must check this before filtering because the caller needs accurate pagination info + // to fetch additional pages when many notifications are filtered out locally. + rawCount := len(allNotifications) + hasNextPage := rawCount >= limit + nextPage := fmt.Sprintf("%d", page+1) + + // Filter by read state if needed + switch readState { + case NotificationStateRead: + // Keep only read notifications + filtered := make([]NotificationData, 0) + for _, n := range allNotifications { + if !n.Unread { + filtered = append(filtered, n) + } + } + allNotifications = filtered + case NotificationStateUnread: + // Keep only unread notifications (API default, but filter just in case) + filtered := make([]NotificationData, 0) + for _, n := range allNotifications { + if n.Unread { + filtered = append(filtered, n) + } + } + allNotifications = filtered + case NotificationStateAll: + // Keep all, no filtering needed + } + + log.Info("Successfully fetched notifications", "rawCount", rawCount, "filteredCount", len(allNotifications), "page", page, "hasNextPage", hasNextPage, "readState", readState) + + return NotificationsResponse{ + Notifications: allNotifications, + TotalCount: len(allNotifications), + PageInfo: PageInfo{ + HasNextPage: hasNextPage, + EndCursor: nextPage, + }, + }, nil +} + +// FetchNotificationByThreadId fetches a single notification by its thread ID. +// This is useful for fetching bookmarked or session-marked-read notifications +// that may not appear in the regular notifications list. +func FetchNotificationByThreadId(threadId string) (*NotificationData, error) { + client, err := getRESTClient() + if err != nil { + return nil, err + } + + path := fmt.Sprintf("notifications/threads/%s", threadId) + log.Debug("Fetching notification by thread ID", "threadId", threadId) + + var notification NotificationData + err = client.Get(path, ¬ification) + if err != nil { + return nil, err + } + + return ¬ification, nil +} + +func MarkNotificationDone(threadId string) error { + client, err := getRESTClient() + if err != nil { + return err + } + + path := fmt.Sprintf("notifications/threads/%s", threadId) + log.Debug("Marking notification as done", "threadId", threadId) + + // DELETE marks as done + err = client.Delete(path, nil) + if err != nil { + return err + } + log.Info("Successfully marked notification as done", "threadId", threadId) + return nil +} + +func MarkNotificationRead(threadId string) error { + client, err := getRESTClient() + if err != nil { + return err + } + + path := fmt.Sprintf("notifications/threads/%s", threadId) + log.Debug("Marking notification as read", "threadId", threadId) + + // PATCH marks as read - returns 205 Reset Content with no body + // The REST client may return an error trying to parse the empty response + err = client.Patch(path, nil, nil) + if err != nil && err.Error() != "unexpected end of JSON input" { + return err + } + log.Info("Successfully marked notification as read", "threadId", threadId) + return nil +} + +func UnsubscribeFromThread(threadId string) error { + client, err := getRESTClient() + if err != nil { + return err + } + + log.Debug("Unsubscribing from notification thread", "threadId", threadId) + + // DELETE /notifications/threads/{thread_id}/subscription + // Mutes all future notifications for a conversation until you comment on the thread or get an @mention + path := fmt.Sprintf("notifications/threads/%s/subscription", threadId) + err = client.Delete(path, nil) + if err != nil && err.Error() != "unexpected end of JSON input" { + return err + } + log.Info("Successfully unsubscribed from thread", "threadId", threadId) + return nil +} + +func MarkAllNotificationsRead() error { + client, err := getRESTClient() + if err != nil { + return err + } + + log.Debug("Marking all notifications as read") + + // PUT /notifications marks all as read - returns 205 Reset Content with no body + // The REST client may return an error trying to parse the empty response + err = client.Put("notifications", nil, nil) + if err != nil && err.Error() != "unexpected end of JSON input" { + return err + } + log.Info("Successfully marked all notifications as read") + return nil +} + +// CommentResponse represents a GitHub comment with author info +type CommentResponse struct { + User struct { + Login string `json:"login"` + } `json:"user"` +} + +// WorkflowRun represents a GitHub Actions workflow run +type WorkflowRun struct { + Id int64 `json:"id"` + Name string `json:"name"` + HtmlUrl string `json:"html_url"` + HeadBranch string `json:"head_branch"` + UpdatedAt time.Time `json:"updated_at"` + Conclusion string `json:"conclusion"` // success, failure, cancelled, etc. +} + +// WorkflowRunsResponse represents the response from the workflow runs API +type WorkflowRunsResponse struct { + TotalCount int `json:"total_count"` + WorkflowRuns []WorkflowRun `json:"workflow_runs"` +} + +// FetchCommentAuthor fetches the author of a comment from its API URL +// apiUrl is like: https://api.github.com/repos/owner/repo/issues/comments/123456 +func FetchCommentAuthor(apiUrl string) (string, error) { + if apiUrl == "" { + return "", nil + } + + client, err := getRESTClient() + if err != nil { + return "", err + } + + // Extract the path from the full URL + const apiPrefix = "https://api.github.com/" + path := apiUrl + if len(apiUrl) > len(apiPrefix) && apiUrl[:len(apiPrefix)] == apiPrefix { + path = apiUrl[len(apiPrefix):] + } + + var response CommentResponse + err = client.Get(path, &response) + if err != nil { + log.Debug("Failed to fetch comment author", "url", apiUrl, "err", err) + return "", err + } + + return response.User.Login, nil +} + +// FindBestWorkflowRunMatch finds the workflow run closest in time to the notification. +// Returns nil if no suitable match is found within the time window. +// Exported for testing. +func FindBestWorkflowRunMatch(runs []WorkflowRun, notificationUpdatedAt time.Time) *WorkflowRun { + if len(runs) == 0 { + return nil + } + + var bestMatch *WorkflowRun + bestDiff := time.Hour * 24 * 365 // Start with a large value + + for i := range runs { + run := &runs[i] + diff := notificationUpdatedAt.Sub(run.UpdatedAt) + if diff < 0 { + diff = -diff + } + + // Prefer runs that are close in time (within a reasonable window) + if diff < bestDiff && diff < time.Hour { + bestDiff = diff + bestMatch = run + } + } + + // If no close match, just return the most recent run + if bestMatch == nil { + bestMatch = &runs[0] + } + + return bestMatch +} + +// FetchRecentWorkflowRun fetches recent workflow runs for a repo and finds the best match +// based on the notification's updated_at timestamp. Returns the HTML URL of the matching run. +// The title parameter is the notification subject title (e.g., "CI / build (push)") +// which may help identify the correct workflow run. +func FetchRecentWorkflowRun(repo string, notificationUpdatedAt time.Time, title string) (string, error) { + client, err := getRESTClient() + if err != nil { + return "", err + } + + // Fetch recent workflow runs (limit to 20 for performance) + path := fmt.Sprintf("repos/%s/actions/runs?per_page=20", repo) + log.Debug("Fetching workflow runs", "repo", repo, "path", path) + + var response WorkflowRunsResponse + err = client.Get(path, &response) + if err != nil { + log.Debug("Failed to fetch workflow runs", "repo", repo, "err", err) + return "", err + } + + if len(response.WorkflowRuns) == 0 { + return "", nil + } + + bestMatch := FindBestWorkflowRunMatch(response.WorkflowRuns, notificationUpdatedAt) + if bestMatch != nil { + log.Debug("Found matching workflow run", "id", bestMatch.Id, "name", bestMatch.Name, "url", bestMatch.HtmlUrl) + return bestMatch.HtmlUrl, nil + } + + return "", nil +} diff --git a/internal/data/notificationapi_test.go b/internal/data/notificationapi_test.go new file mode 100644 index 000000000..eb1d0e368 --- /dev/null +++ b/internal/data/notificationapi_test.go @@ -0,0 +1,111 @@ +package data + +import ( + "testing" + "time" +) + +func TestFindBestWorkflowRunMatch(t *testing.T) { + baseTime := time.Date(2024, 1, 15, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + runs []WorkflowRun + notificationUpdatedAt time.Time + expectedId int64 + expectedNil bool + }{ + { + name: "empty runs returns nil", + runs: []WorkflowRun{}, + notificationUpdatedAt: baseTime, + expectedNil: true, + }, + { + name: "single run within time window is selected", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(-5 * time.Minute)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 1, + }, + { + name: "closest run within time window is selected", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(-30 * time.Minute)}, + {Id: 2, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/2", UpdatedAt: baseTime.Add(-5 * time.Minute)}, + {Id: 3, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/3", UpdatedAt: baseTime.Add(-15 * time.Minute)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 2, // 5 minutes is closest + }, + { + name: "run slightly after notification time is selected", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(2 * time.Minute)}, + {Id: 2, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/2", UpdatedAt: baseTime.Add(-10 * time.Minute)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 1, // 2 minutes after is closer than 10 minutes before + }, + { + name: "no runs within time window falls back to first run", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(-2 * time.Hour)}, + {Id: 2, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/2", UpdatedAt: baseTime.Add(-3 * time.Hour)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 1, // Falls back to first (most recent) run + }, + { + name: "exact time match is selected", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(-10 * time.Minute)}, + {Id: 2, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/2", UpdatedAt: baseTime}, + {Id: 3, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/3", UpdatedAt: baseTime.Add(-5 * time.Minute)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 2, // Exact match + }, + { + name: "run at edge of time window (59 minutes) is selected", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(-59 * time.Minute)}, + {Id: 2, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/2", UpdatedAt: baseTime.Add(-61 * time.Minute)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 1, // 59 minutes is within the 1 hour window + }, + { + name: "notification time before all runs still finds closest", + runs: []WorkflowRun{ + {Id: 1, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/1", UpdatedAt: baseTime.Add(30 * time.Minute)}, + {Id: 2, Name: "CI", HtmlUrl: "https://github.com/owner/repo/actions/runs/2", UpdatedAt: baseTime.Add(10 * time.Minute)}, + }, + notificationUpdatedAt: baseTime, + expectedId: 2, // 10 minutes after is closer + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FindBestWorkflowRunMatch(tt.runs, tt.notificationUpdatedAt) + + if tt.expectedNil { + if result != nil { + t.Errorf("FindBestWorkflowRunMatch() = %v, want nil", result) + } + return + } + + if result == nil { + t.Errorf("FindBestWorkflowRunMatch() = nil, want run with id %d", tt.expectedId) + return + } + + if result.Id != tt.expectedId { + t.Errorf("FindBestWorkflowRunMatch() returned run id %d, want %d", result.Id, tt.expectedId) + } + }) + } +} diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 517d9ac4f..03361ec9f 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -26,8 +26,32 @@ type SuggestedReviewer struct { } type EnrichedPullRequestData struct { - Url string - Number int + Url string + Number int + Title string + Body string + State string + IsDraft bool + Author struct { + Login string + } + AuthorAssociation string + UpdatedAt time.Time + CreatedAt time.Time + Mergeable string + ReviewDecision string + Additions int + Deletions int + HeadRefName string + BaseRefName string + HeadRepository struct { + Name string + } + HeadRef struct { + Name string + } + Labels PRLabels `graphql:"labels(first: 6)"` + Assignees Assignees `graphql:"assignees(first: 3)"` Repository Repository Commits LastCommitWithStatusChecks `graphql:"commits(last: 1)"` AllCommits AllCommits `graphql:"allCommits: commits(last: 100)"` @@ -36,6 +60,7 @@ type EnrichedPullRequestData struct { ReviewRequests ReviewRequests `graphql:"reviewRequests(last: 100)"` Reviews Reviews `graphql:"reviews(last: 100)"` SuggestedReviewers []SuggestedReviewer + Files ChangedFiles `graphql:"files(first: 5)"` } type PullRequestData struct { @@ -371,6 +396,38 @@ func (data PullRequestData) GetCreatedAt() time.Time { return data.CreatedAt } +// ToPullRequestData converts EnrichedPullRequestData to PullRequestData +// This is useful when we fetch a single PR and need basic PR fields +func (e EnrichedPullRequestData) ToPullRequestData() PullRequestData { + return PullRequestData{ + Number: e.Number, + Title: e.Title, + Body: e.Body, + Author: e.Author, + AuthorAssociation: e.AuthorAssociation, + UpdatedAt: e.UpdatedAt, + CreatedAt: e.CreatedAt, + Url: e.Url, + State: e.State, + Mergeable: e.Mergeable, + ReviewDecision: e.ReviewDecision, + Additions: e.Additions, + Deletions: e.Deletions, + HeadRefName: e.HeadRefName, + BaseRefName: e.BaseRefName, + HeadRepository: e.HeadRepository, + HeadRef: e.HeadRef, + Repository: e.Repository, + Assignees: e.Assignees, + IsDraft: e.IsDraft, + Labels: e.Labels, + Files: e.Files, + // Note: Comments, ReviewThreads, Reviews, ReviewRequests, Commits + // have different types in EnrichedPullRequestData vs PullRequestData + // We leave them as zero values since the enriched data will be used instead + } +} + func makePullRequestsQuery(query string) string { return fmt.Sprintf("is:pr %s sort:updated", query) } diff --git a/internal/tui/common/diff.go b/internal/tui/common/diff.go new file mode 100644 index 000000000..d484d27ac --- /dev/null +++ b/internal/tui/common/diff.go @@ -0,0 +1,31 @@ +package common + +import ( + "fmt" + "os/exec" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" +) + +// DiffPR opens a diff view for a PR using the gh CLI. +// The env parameter should be the result of Config.GetFullScreenDiffPagerEnv(). +func DiffPR(prNumber int, repoName string, env []string) tea.Cmd { + c := exec.Command( + "gh", + "pr", + "diff", + fmt.Sprint(prNumber), + "-R", + repoName, + ) + c.Env = env + + return tea.ExecProcess(c, func(err error) tea.Msg { + if err != nil { + return constants.ErrMsg{Err: err} + } + return nil + }) +} diff --git a/internal/tui/common/diff_test.go b/internal/tui/common/diff_test.go new file mode 100644 index 000000000..0aee39393 --- /dev/null +++ b/internal/tui/common/diff_test.go @@ -0,0 +1,46 @@ +package common + +import ( + "testing" +) + +func TestDiffPR(t *testing.T) { + tests := []struct { + name string + prNumber int + repoName string + wantNil bool + }{ + { + name: "returns command for valid PR", + prNumber: 123, + repoName: "owner/repo", + wantNil: false, + }, + { + name: "returns command for PR number 0", + prNumber: 0, + repoName: "owner/repo", + wantNil: false, + }, + { + name: "returns command with hyphenated repo name", + prNumber: 456, + repoName: "my-org/my-repo", + wantNil: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := DiffPR(tt.prNumber, tt.repoName, nil) + + if tt.wantNil && cmd != nil { + t.Errorf("DiffPR() returned non-nil, want nil") + } + if !tt.wantNil && cmd == nil { + t.Errorf("DiffPR() returned nil, want non-nil") + } + }) + } +} diff --git a/internal/tui/common/styles.go b/internal/tui/common/styles.go index 10cc28b69..28850a606 100644 --- a/internal/tui/common/styles.go +++ b/internal/tui/common/styles.go @@ -82,3 +82,25 @@ func BuildStyles(theme theme.Theme) CommonStyles { Render(constants.MergedIcon) return s } + +// RenderPreviewHeader renders the repo/number line at the top of preview panes +// (e.g., "owner/repo · #123" or "#123 · owner/repo") +func RenderPreviewHeader(theme theme.Theme, width int, text string) string { + return lipgloss.NewStyle(). + PaddingLeft(1). + Width(width). + Background(theme.SelectedBackground). + Foreground(theme.SecondaryText). + Render(text) +} + +// RenderPreviewTitle renders the title block with background highlight +func RenderPreviewTitle(theme theme.Theme, styles CommonStyles, width int, title string) string { + return lipgloss.NewStyle().Height(3).Width(width).Background( + theme.SelectedBackground).PaddingLeft(1).Render( + lipgloss.PlaceVertical(3, lipgloss.Center, styles.MainTextStyle. + Background(theme.SelectedBackground). + Render(title), + ), + ) +} diff --git a/internal/tui/components/footer/footer.go b/internal/tui/components/footer/footer.go index fa87f881f..172cd4468 100644 --- a/internal/tui/components/footer/footer.go +++ b/internal/tui/components/footer/footer.go @@ -17,6 +17,8 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/utils" ) +const viewSeparator = " │ " + type Model struct { ctx *context.ProgramContext leftSection *string @@ -109,15 +111,49 @@ func (m *Model) UpdateProgramContext(ctx *context.ProgramContext) { } func (m *Model) renderViewButton(view config.ViewType) string { - v := " PRs" - if view == config.IssuesView { - v = " Issues" + isActive := m.ctx.View == view + + // Define icons and labels for each view + var icon, label string + // Define icons - notifications has solid/outline variants + solidBell := "" + outlineBell := "" + + switch view { + case config.NotificationsView: + if m.ctx.View == config.NotificationsView { + icon = solidBell + } else { + icon = outlineBell + } + label = "" + case config.PRsView: + icon = "" + label = " PRs" + case config.IssuesView: + icon = "" + label = " Issues" } - if m.ctx.View == view { - return m.ctx.Styles.ViewSwitcher.ActiveView.Render(v) + if isActive { + // Active: colored icon + prominent background + // Use gold for notifications bell, green for others + iconColor := m.ctx.Theme.SuccessText + if view == config.NotificationsView { + iconColor = lipgloss.AdaptiveColor{Light: "#B8860B", Dark: "#FFD700"} // Gold + } + activeStyle := lipgloss.NewStyle(). + Foreground(iconColor). + Background(m.ctx.Styles.ViewSwitcher.ActiveView.GetBackground()). + Bold(true) + if label != "" { + return activeStyle.Render(icon) + activeStyle.Render(label) + } + return activeStyle.Render(icon) } - return m.ctx.Styles.ViewSwitcher.InactiveView.Render(v) + + // Inactive: faint styling + return m.ctx.Styles.ViewSwitcher.InactiveView.Render(icon + label) } func (m *Model) renderViewSwitcher(ctx *context.ProgramContext) string { @@ -137,8 +173,10 @@ func (m *Model) renderViewSwitcher(ctx *context.ProgramContext) string { view := lipgloss.JoinHorizontal( lipgloss.Top, - ctx.Styles.ViewSwitcher.ViewsSeparator.PaddingLeft(1).Render(m.renderViewButton(config.PRsView)), - ctx.Styles.ViewSwitcher.ViewsSeparator.Render(" │ "), + ctx.Styles.ViewSwitcher.ViewsSeparator.PaddingLeft(1).Render(m.renderViewButton(config.NotificationsView)), + ctx.Styles.ViewSwitcher.ViewsSeparator.Render(viewSeparator), + m.renderViewButton(config.PRsView), + ctx.Styles.ViewSwitcher.ViewsSeparator.Render(viewSeparator), m.renderViewButton(config.IssuesView), lipgloss.NewStyle().Background(ctx.Styles.Common.FooterStyle.GetBackground()).Foreground( ctx.Styles.ViewSwitcher.ViewsSeparator.GetBackground()).Render(" "), diff --git a/internal/tui/components/issueview/action.go b/internal/tui/components/issueview/action.go new file mode 100644 index 000000000..19a4a8ea2 --- /dev/null +++ b/internal/tui/components/issueview/action.go @@ -0,0 +1,19 @@ +package issueview + +// IssueActionType identifies the type of action requested by a key press in the issue view. +type IssueActionType int + +const ( + IssueActionNone IssueActionType = iota + IssueActionLabel + IssueActionAssign + IssueActionUnassign + IssueActionComment + IssueActionClose + IssueActionReopen +) + +// IssueAction represents an action to be performed on an issue. +type IssueAction struct { + Type IssueActionType +} diff --git a/internal/tui/components/issueview/action_test.go b/internal/tui/components/issueview/action_test.go new file mode 100644 index 000000000..9e9737b6b --- /dev/null +++ b/internal/tui/components/issueview/action_test.go @@ -0,0 +1,158 @@ +package issueview + +import ( + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/require" + + "github.com/dlvhdr/gh-dash/v4/internal/config" + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/issuerow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/keys" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" +) + +func newTestModelForAction(t *testing.T) Model { + t.Helper() + cfg, err := config.ParseConfig(config.Location{ + ConfigFlag: "../../../config/testdata/test-config.yml", + }) + if err != nil { + t.Fatal(err) + } + thm := theme.ParseTheme(&cfg) + ctx := &context.ProgramContext{ + Config: &cfg, + Theme: thm, + Styles: context.InitStyles(thm), + } + + m := NewModel(ctx) + m.ctx = ctx + m.issue = &issuerow.Issue{ + Ctx: ctx, + Data: data.IssueData{}, + } + return m +} + +func TestUpdateReturnsCorrectActions(t *testing.T) { + testCases := []struct { + name string + keyBinding string + expectedAction IssueActionType + }{ + {"label key", "L", IssueActionLabel}, + // Note: IssueKeys.Assign has no default binding in issueKeys.go + {"unassign key", "A", IssueActionUnassign}, + {"comment key", "c", IssueActionComment}, + {"close key", "x", IssueActionClose}, + {"reopen key", "X", IssueActionReopen}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + m := newTestModelForAction(t) + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(tc.keyBinding)} + + _, _, action := m.Update(msg) + + require.NotNil(t, action, "expected action for key %q", tc.keyBinding) + require.Equal(t, tc.expectedAction, action.Type, + "expected action type %v for key %q, got %v", tc.expectedAction, tc.keyBinding, action.Type) + }) + } +} + +func TestUpdateReturnsNilActionForUnknownKeys(t *testing.T) { + m := newTestModelForAction(t) + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("z")} + + _, _, action := m.Update(msg) + + require.Nil(t, action, "expected nil action for unknown key") +} + +func TestUpdateReturnsNilActionWhenCommenting(t *testing.T) { + m := newTestModelForAction(t) + m.isCommenting = true + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("L")} // label key + + _, _, action := m.Update(msg) + + require.Nil(t, action, "expected nil action when in commenting mode") +} + +func TestUpdateReturnsNilActionWhenLabeling(t *testing.T) { + m := newTestModelForAction(t) + m.isLabeling = true + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("c")} // comment key + + _, _, action := m.Update(msg) + + require.Nil(t, action, "expected nil action when in labeling mode") +} + +func TestUpdateReturnsNilActionWhenAssigning(t *testing.T) { + m := newTestModelForAction(t) + m.isAssigning = true + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("x")} // close key + + _, _, action := m.Update(msg) + + require.Nil(t, action, "expected nil action when in assigning mode") +} + +func TestUpdateReturnsNilActionWhenUnassigning(t *testing.T) { + m := newTestModelForAction(t) + m.isUnassigning = true + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("X")} // reopen key + + _, _, action := m.Update(msg) + + require.Nil(t, action, "expected nil action when in unassigning mode") +} + +func TestIssueActionTypes(t *testing.T) { + // Verify all action types are distinct + actionTypes := []IssueActionType{ + IssueActionNone, + IssueActionLabel, + IssueActionAssign, + IssueActionUnassign, + IssueActionComment, + IssueActionClose, + IssueActionReopen, + } + + seen := make(map[IssueActionType]bool) + for _, at := range actionTypes { + require.False(t, seen[at], "duplicate action type value: %v", at) + seen[at] = true + } + + // Verify IssueActionNone is zero value + require.Equal(t, IssueActionType(0), IssueActionNone, "IssueActionNone should be zero value") +} + +func TestUpdateWithReboundKeys(t *testing.T) { + // Save original key bindings + originalLabelKeys := keys.IssueKeys.Label.Keys() + + // Rebind label key to "l" (lowercase) + keys.IssueKeys.Label.SetKeys("l") + defer func() { + // Restore original bindings + keys.IssueKeys.Label.SetKeys(originalLabelKeys...) + }() + + m := newTestModelForAction(t) + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("l")} + + _, _, action := m.Update(msg) + + require.NotNil(t, action, "expected action for rebound key") + require.Equal(t, IssueActionLabel, action.Type, "expected label action for rebound key") +} diff --git a/internal/tui/components/issueview/activity.go b/internal/tui/components/issueview/activity.go index 022c0a6e3..a79d6a836 100644 --- a/internal/tui/components/issueview/activity.go +++ b/internal/tui/components/issueview/activity.go @@ -64,11 +64,16 @@ func renderEmptyState() string { } func (m *Model) renderComment(comment data.IssueComment, markdownRenderer glamour.TermRenderer) (string, error) { - header := lipgloss.JoinHorizontal(lipgloss.Top, - m.ctx.Styles.Common.MainTextStyle.Render(comment.Author.Login), - " ", - lipgloss.NewStyle().Foreground(m.ctx.Theme.FaintText).Render(utils.TimeElapsed(comment.UpdatedAt)), - ) + width := m.getIndentedContentWidth() - 2 + header := lipgloss.NewStyle(). + Width(width). + BorderStyle(lipgloss.RoundedBorder()). + BorderForeground(m.ctx.Theme.FaintBorder).Render( + lipgloss.JoinHorizontal(lipgloss.Top, + m.ctx.Styles.Common.MainTextStyle.Render(comment.Author.Login), + " ", + lipgloss.NewStyle().Foreground(m.ctx.Theme.FaintText).Render(utils.TimeElapsed(comment.UpdatedAt)), + )) body := lineCleanupRegex.ReplaceAllString(comment.Body, "") body, err := markdownRenderer.Render(body) diff --git a/internal/tui/components/issueview/issueview.go b/internal/tui/components/issueview/issueview.go index 3946825d0..f3857b72b 100644 --- a/internal/tui/components/issueview/issueview.go +++ b/internal/tui/components/issueview/issueview.go @@ -17,7 +17,9 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/components/inputbox" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/issuerow" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/keys" "github.com/dlvhdr/gh-dash/v4/internal/tui/markdown" + "github.com/dlvhdr/gh-dash/v4/internal/utils" ) var ( @@ -77,7 +79,7 @@ func NewModel(ctx *context.ProgramContext) Model { } } -func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { +func (m Model) Update(msg tea.Msg) (Model, tea.Cmd, *IssueAction) { var ( cmds []tea.Cmd cmd tea.Cmd @@ -96,11 +98,11 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { existingLabels := allLabels(m.inputBox.Value()) m.ac.Show(currentLabel, existingLabels) } - return m, clearCmd + return m, clearCmd, nil case RepoLabelsFetchFailedMsg: clearCmd := m.ac.SetFetchError(msg.Err) - return m, clearCmd + return m, clearCmd, nil case autocomplete.FetchSuggestionsRequestedMsg: // Only fetch when we're in labeling mode (where labels are relevant) @@ -114,9 +116,9 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { } } cmd := m.fetchLabels() - return m, cmd + return m, cmd, nil } - return m, nil + return m, nil, nil case tea.KeyMsg: if m.isCommenting { @@ -127,7 +129,7 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { } m.inputBox.Blur() m.isCommenting = false - return m, cmd + return m, cmd, nil case tea.KeyEsc, tea.KeyCtrlC: if !m.ShowConfirmCancel { @@ -136,13 +138,13 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { default: if msg.String() == "Y" || msg.String() == "y" { if m.shouldCancelComment() { - return m, nil + return m, nil, nil } } if m.ShowConfirmCancel && (msg.String() == "N" || msg.String() == "n") { m.inputBox.SetPrompt(commentPrompt) m.ShowConfirmCancel = false - return m, nil + return m, nil, nil } m.inputBox.SetPrompt(commentPrompt) m.ShowConfirmCancel = false @@ -160,13 +162,13 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { m.inputBox.Blur() m.isLabeling = false m.ac.Hide() - return m, cmd + return m, cmd, nil case tea.KeyEsc, tea.KeyCtrlC: m.inputBox.Blur() m.isLabeling = false m.ac.Hide() - return m, nil + return m, nil, nil } if key.Matches(msg, autocomplete.RefreshSuggestionsKey) { @@ -201,12 +203,12 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { } m.inputBox.Blur() m.isAssigning = false - return m, cmd + return m, cmd, nil case tea.KeyEsc, tea.KeyCtrlC: m.inputBox.Blur() m.isAssigning = false - return m, nil + return m, nil, nil } m.inputBox, taCmd = m.inputBox.Update(msg) @@ -220,18 +222,32 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { } m.inputBox.Blur() m.isUnassigning = false - return m, cmd + return m, cmd, nil case tea.KeyEsc, tea.KeyCtrlC: m.inputBox.Blur() m.isUnassigning = false - return m, nil + return m, nil, nil } m.inputBox, taCmd = m.inputBox.Update(msg) cmds = append(cmds, cmd, taCmd) } else { - return m, nil + switch { + case key.Matches(msg, keys.IssueKeys.Label): + return m, nil, &IssueAction{Type: IssueActionLabel} + case key.Matches(msg, keys.IssueKeys.Assign): + return m, nil, &IssueAction{Type: IssueActionAssign} + case key.Matches(msg, keys.IssueKeys.Unassign): + return m, nil, &IssueAction{Type: IssueActionUnassign} + case key.Matches(msg, keys.IssueKeys.Comment): + return m, nil, &IssueAction{Type: IssueActionComment} + case key.Matches(msg, keys.IssueKeys.Close): + return m, nil, &IssueAction{Type: IssueActionClose} + case key.Matches(msg, keys.IssueKeys.Reopen): + return m, nil, &IssueAction{Type: IssueActionReopen} + } + return m, nil, nil } } @@ -242,7 +258,7 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { cmds = append(cmds, acCmd) } - return m, tea.Batch(cmds...) + return m, tea.Batch(cmds...), nil } func (m Model) View() string { @@ -255,6 +271,8 @@ func (m Model) View() string { s.WriteString("\n\n") s.WriteString(m.renderStatusPill()) s.WriteString("\n\n") + s.WriteString(m.renderAuthor()) + s.WriteString("\n\n") labels := m.renderLabels() if labels != "" { @@ -276,14 +294,12 @@ func (m Model) View() string { } func (m *Model) renderFullNameAndNumber() string { - return lipgloss.NewStyle(). - Foreground(m.ctx.Theme.SecondaryText). - Render(fmt.Sprintf("#%d · %s", m.issue.Data.GetNumber(), m.issue.Data.GetRepoNameWithOwner())) + return common.RenderPreviewHeader(m.ctx.Theme, m.width, + fmt.Sprintf("#%d · %s", m.issue.Data.GetNumber(), m.issue.Data.GetRepoNameWithOwner())) } func (m *Model) renderTitle() string { - return m.ctx.Styles.Common.MainTextStyle.Width(m.getIndentedContentWidth()). - Render(m.issue.Data.Title) + return common.RenderPreviewTitle(m.ctx.Theme, m.ctx.Styles.Common, m.width, m.issue.Data.Title) } func (m *Model) renderStatusPill() string { @@ -304,6 +320,25 @@ func (m *Model) renderStatusPill() string { Render(content) } +func (m *Model) renderAuthor() string { + authorAssociation := m.issue.Data.AuthorAssociation + if authorAssociation == "" { + authorAssociation = "unknown role" + } + time := lipgloss.NewStyle().Render(utils.TimeElapsed(m.issue.Data.CreatedAt)) + return lipgloss.JoinHorizontal(lipgloss.Top, + " by ", + lipgloss.NewStyle().Foreground(m.ctx.Theme.PrimaryText).Render( + lipgloss.NewStyle().Bold(true).Render("@"+m.issue.Data.Author.Login)), + lipgloss.NewStyle().Foreground(m.ctx.Theme.FaintText).Render( + lipgloss.JoinHorizontal(lipgloss.Top, " ⋅ ", time, " ago", " ⋅ ")), + lipgloss.NewStyle().Foreground(m.ctx.Theme.FaintText).Render( + lipgloss.JoinHorizontal(lipgloss.Top, data.GetAuthorRoleIcon(m.issue.Data.AuthorAssociation, + m.ctx.Theme), " ", lipgloss.NewStyle().Foreground(m.ctx.Theme.FaintText).Render(strings.ToLower(authorAssociation))), + ), + ) +} + func (m *Model) renderBody() string { width := m.getIndentedContentWidth() // Strip HTML comments from body and cleanup body. diff --git a/internal/tui/components/listviewport/listviewport.go b/internal/tui/components/listviewport/listviewport.go index b8c71b0fd..2b06c2060 100644 --- a/internal/tui/components/listviewport/listviewport.go +++ b/internal/tui/components/listviewport/listviewport.go @@ -63,6 +63,10 @@ func (m *Model) SetTotalItems(total int) { m.NumTotalItems = total } +func (m *Model) SetItemHeight(height int) { + m.ListItemHeight = height +} + func (m *Model) SyncViewPort(content string) { m.viewport.SetContent(content) } diff --git a/internal/tui/components/notificationrow/data.go b/internal/tui/components/notificationrow/data.go new file mode 100644 index 000000000..0bd52cdb3 --- /dev/null +++ b/internal/tui/components/notificationrow/data.go @@ -0,0 +1,189 @@ +package notificationrow + +import ( + "fmt" + "strconv" + "strings" + "time" + + "github.com/dlvhdr/gh-dash/v4/internal/data" +) + +// PR/Issue state constants from GitHub API +const ( + StateOpen = "OPEN" + StateClosed = "CLOSED" + StateMerged = "MERGED" +) + +type Data struct { + Notification data.NotificationData + NewCommentsCount int // Number of new comments since last read + SubjectState string // State of the PR/Issue (OPEN, CLOSED, MERGED) + IsDraft bool // Whether PR is a draft + Actor string // Username of the user who triggered the notification + ActivityDescription string // Human-readable description of the activity (e.g., "@user commented on this PR") + ResolvedUrl string // Async-resolved URL (e.g., for CheckSuite -> specific workflow run) +} + +func (d Data) GetTitle() string { + // Sanitize title: remove carriage returns and other control characters + // that can corrupt terminal rendering (e.g., GitHub sometimes returns + // titles with trailing \r characters) + title := d.Notification.Subject.Title + title = strings.ReplaceAll(title, "\r", "") + title = strings.ReplaceAll(title, "\n", " ") + return strings.TrimSpace(title) +} + +func (d Data) GetRepoNameWithOwner() string { + return d.Notification.Repository.FullName +} + +func (d Data) GetNumber() int { + subject := d.Notification.Subject + if subject.Type == "PullRequest" || subject.Type == "Issue" { + numStr := extractNumberFromUrl(subject.Url) + if num, err := strconv.Atoi(numStr); err == nil { + return num + } + } + return 0 +} + +func (d Data) GetUrl() string { + subject := d.Notification.Subject + repo := d.Notification.Repository.FullName + + switch subject.Type { + case "PullRequest": + return fmt.Sprintf("https://github.com/%s/pull/%s", repo, extractNumberFromUrl(subject.Url)) + case "Issue": + return fmt.Sprintf("https://github.com/%s/issues/%s", repo, extractNumberFromUrl(subject.Url)) + case "Discussion": + num := extractNumberFromUrl(subject.Url) + if num != "" { + return fmt.Sprintf("https://github.com/%s/discussions/%s", repo, num) + } + return fmt.Sprintf("https://github.com/%s/discussions", repo) + case "Release": + return fmt.Sprintf("https://github.com/%s/releases", repo) + case "Commit": + return fmt.Sprintf("https://github.com/%s/commits", repo) + case "CheckSuite": + // GitHub's API returns subject.url=null for CheckSuite notifications. + // ResolvedUrl is populated asynchronously with the specific workflow run URL. + // Until resolved, we fall back to the repository's actions page. + if d.ResolvedUrl != "" { + return d.ResolvedUrl + } + return fmt.Sprintf("https://github.com/%s/actions", repo) + default: + return fmt.Sprintf("https://github.com/%s", repo) + } +} + +func (d Data) GetUpdatedAt() time.Time { + return d.Notification.UpdatedAt +} + +func (d Data) GetCreatedAt() time.Time { + return d.Notification.UpdatedAt +} + +func (d Data) GetId() string { + return d.Notification.Id +} + +func (d Data) GetSubjectType() string { + return d.Notification.Subject.Type +} + +func (d Data) GetReason() string { + return d.Notification.Reason +} + +func (d Data) IsUnread() bool { + return d.Notification.Unread +} + +func (d Data) GetLatestCommentUrl() string { + return d.Notification.Subject.LatestCommentUrl +} + +// extractNumberFromUrl extracts the last path segment (typically a number) from an API URL +func extractNumberFromUrl(apiUrl string) string { + if apiUrl == "" { + return "" + } + for i := len(apiUrl) - 1; i >= 0; i-- { + if apiUrl[i] == '/' { + return apiUrl[i+1:] + } + } + return "" +} + +// GenerateActivityDescription creates a human-readable description of the notification activity +func GenerateActivityDescription(reason, subjectType, actor string) string { + switch reason { + case "comment": + if actor != "" { + switch subjectType { + case "PullRequest": + return fmt.Sprintf("@%s commented on this pull request", actor) + case "Issue": + return fmt.Sprintf("@%s commented on this issue", actor) + default: + return fmt.Sprintf("@%s commented", actor) + } + } + return "New comment" + case "review_requested": + if actor != "" { + return fmt.Sprintf("@%s requested your review", actor) + } + return "Review requested" + case "mention": + if actor != "" { + return fmt.Sprintf("@%s mentioned you", actor) + } + return "You were mentioned" + case "author": + return "Activity on your thread" + case "assign": + return "You were assigned" + case "state_change": + switch subjectType { + case "PullRequest": + return "Pull request state changed" + case "Issue": + return "Issue state changed" + default: + return "State changed" + } + case "ci_activity": + return "CI activity" + case "subscribed": + if actor != "" { + switch subjectType { + case "PullRequest": + return fmt.Sprintf("@%s commented on this pull request", actor) + case "Issue": + return fmt.Sprintf("@%s commented on this issue", actor) + default: + return "Activity on subscribed thread" + } + } + return "Activity on subscribed thread" + case "team_mention": + return "Your team was mentioned" + case "security_alert": + return "Security vulnerability detected" + default: + if actor != "" { + return fmt.Sprintf("@%s triggered this notification", actor) + } + return "" + } +} diff --git a/internal/tui/components/notificationrow/data_test.go b/internal/tui/components/notificationrow/data_test.go new file mode 100644 index 000000000..26155e7e7 --- /dev/null +++ b/internal/tui/components/notificationrow/data_test.go @@ -0,0 +1,866 @@ +package notificationrow + +import ( + "testing" + + "github.com/dlvhdr/gh-dash/v4/internal/data" +) + +func TestExtractNumberFromUrl(t *testing.T) { + tests := []struct { + name string + apiUrl string + expected string + }{ + { + name: "empty URL returns empty string", + apiUrl: "", + expected: "", + }, + { + name: "PR API URL extracts number", + apiUrl: "https://api.github.com/repos/owner/repo/pulls/123", + expected: "123", + }, + { + name: "Issue API URL extracts number", + apiUrl: "https://api.github.com/repos/owner/repo/issues/456", + expected: "456", + }, + { + name: "Discussion API URL extracts number", + apiUrl: "https://api.github.com/repos/owner/repo/discussions/789", + expected: "789", + }, + { + name: "URL with no slashes returns empty", + apiUrl: "no-slashes", + expected: "", + }, + { + name: "URL ending with slash returns empty", + apiUrl: "https://api.github.com/repos/owner/repo/", + expected: "", + }, + { + name: "single segment after slash", + apiUrl: "/123", + expected: "123", + }, + { + name: "large number", + apiUrl: "https://api.github.com/repos/owner/repo/pulls/999999", + expected: "999999", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractNumberFromUrl(tt.apiUrl) + if result != tt.expected { + t.Errorf("extractNumberFromUrl(%q) = %q, want %q", tt.apiUrl, result, tt.expected) + } + }) + } +} + +func TestGetNumber(t *testing.T) { + tests := []struct { + name string + data Data + expected int + }{ + { + name: "PullRequest returns extracted number", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "PullRequest", + Url: "https://api.github.com/repos/owner/repo/pulls/123", + }, + }, + }, + expected: 123, + }, + { + name: "Issue returns extracted number", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Issue", + Url: "https://api.github.com/repos/owner/repo/issues/456", + }, + }, + }, + expected: 456, + }, + { + name: "Discussion returns 0", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Discussion", + Url: "https://api.github.com/repos/owner/repo/discussions/789", + }, + }, + }, + expected: 0, + }, + { + name: "Release returns 0", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Release", + Url: "https://api.github.com/repos/owner/repo/releases/v1.0.0", + }, + }, + }, + expected: 0, + }, + { + name: "empty URL returns 0", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "PullRequest", + Url: "", + }, + }, + }, + expected: 0, + }, + { + name: "non-numeric segment returns 0", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "PullRequest", + Url: "https://api.github.com/repos/owner/repo/pulls/abc", + }, + }, + }, + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetNumber() + if result != tt.expected { + t.Errorf("GetNumber() = %d, want %d", result, tt.expected) + } + }) + } +} + +func TestGetUrl(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "PullRequest constructs correct URL", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "PullRequest", + Url: "https://api.github.com/repos/owner/repo/pulls/123", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/pull/123", + }, + { + name: "Issue constructs correct URL", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Issue", + Url: "https://api.github.com/repos/owner/repo/issues/456", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/issues/456", + }, + { + name: "Discussion with number constructs correct URL", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Discussion", + Url: "https://api.github.com/repos/owner/repo/discussions/789", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/discussions/789", + }, + { + name: "Discussion with empty URL falls back to discussions page", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Discussion", + Url: "", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/discussions", + }, + { + name: "Release constructs releases URL", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Release", + Url: "https://api.github.com/repos/owner/repo/releases/12345", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/releases", + }, + { + name: "Commit constructs commits URL", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Commit", + Url: "https://api.github.com/repos/owner/repo/commits/abc123", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/commits", + }, + { + name: "CheckSuite links to actions page when ResolvedUrl not set", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "CheckSuite", + Url: "", // GitHub API returns null for CheckSuite subject.url + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo/actions", + }, + { + name: "CheckSuite uses ResolvedUrl when available", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "CheckSuite", + Url: "", // GitHub API returns null for CheckSuite subject.url + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + ResolvedUrl: "https://github.com/owner/repo/actions/runs/12345678", + }, + expected: "https://github.com/owner/repo/actions/runs/12345678", + }, + { + name: "unknown type falls back to repo URL", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "UnknownType", + Url: "https://api.github.com/repos/owner/repo/something", + }, + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "https://github.com/owner/repo", + }, + { + name: "handles org/repo with special characters", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "PullRequest", + Url: "https://api.github.com/repos/my-org/my-repo/pulls/1", + }, + Repository: data.NotificationRepository{ + FullName: "my-org/my-repo", + }, + }, + }, + expected: "https://github.com/my-org/my-repo/pull/1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetUrl() + if result != tt.expected { + t.Errorf("GetUrl() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestIsUnread(t *testing.T) { + tests := []struct { + name string + data Data + expected bool + }{ + { + name: "unread notification returns true", + data: Data{ + Notification: data.NotificationData{ + Unread: true, + }, + }, + expected: true, + }, + { + name: "read notification returns false", + data: Data{ + Notification: data.NotificationData{ + Unread: false, + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.IsUnread() + if result != tt.expected { + t.Errorf("IsUnread() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestGetReason(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "returns comment reason", + data: Data{ + Notification: data.NotificationData{ + Reason: "comment", + }, + }, + expected: "comment", + }, + { + name: "returns mention reason", + data: Data{ + Notification: data.NotificationData{ + Reason: "mention", + }, + }, + expected: "mention", + }, + { + name: "returns subscribed reason", + data: Data{ + Notification: data.NotificationData{ + Reason: "subscribed", + }, + }, + expected: "subscribed", + }, + { + name: "returns review_requested reason", + data: Data{ + Notification: data.NotificationData{ + Reason: "review_requested", + }, + }, + expected: "review_requested", + }, + { + name: "returns empty string when not set", + data: Data{ + Notification: data.NotificationData{ + Reason: "", + }, + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetReason() + if result != tt.expected { + t.Errorf("GetReason() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestGetLatestCommentUrl(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "returns latest comment URL when set", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + LatestCommentUrl: "https://api.github.com/repos/owner/repo/issues/comments/123456", + }, + }, + }, + expected: "https://api.github.com/repos/owner/repo/issues/comments/123456", + }, + { + name: "returns empty string when not set", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + LatestCommentUrl: "", + }, + }, + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetLatestCommentUrl() + if result != tt.expected { + t.Errorf("GetLatestCommentUrl() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestGetTitle(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "returns subject title", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Title: "Fix bug in authentication", + }, + }, + }, + expected: "Fix bug in authentication", + }, + { + name: "returns empty string when not set", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Title: "", + }, + }, + }, + expected: "", + }, + { + name: "strips trailing carriage return", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Title: "Set creation URL of worker settings\r", + }, + }, + }, + expected: "Set creation URL of worker settings", + }, + { + name: "strips embedded carriage returns", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Title: "Line one\r\nLine two", + }, + }, + }, + expected: "Line one Line two", + }, + { + name: "trims whitespace", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Title: " Title with spaces ", + }, + }, + }, + expected: "Title with spaces", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetTitle() + if result != tt.expected { + t.Errorf("GetTitle() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestGetRepoNameWithOwner(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "returns full repository name", + data: Data{ + Notification: data.NotificationData{ + Repository: data.NotificationRepository{ + FullName: "owner/repo", + }, + }, + }, + expected: "owner/repo", + }, + { + name: "returns org/repo format", + data: Data{ + Notification: data.NotificationData{ + Repository: data.NotificationRepository{ + FullName: "my-organization/my-repository", + }, + }, + }, + expected: "my-organization/my-repository", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetRepoNameWithOwner() + if result != tt.expected { + t.Errorf("GetRepoNameWithOwner() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestGetSubjectType(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "returns PullRequest type", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "PullRequest", + }, + }, + }, + expected: "PullRequest", + }, + { + name: "returns Issue type", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Issue", + }, + }, + }, + expected: "Issue", + }, + { + name: "returns Discussion type", + data: Data{ + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: "Discussion", + }, + }, + }, + expected: "Discussion", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetSubjectType() + if result != tt.expected { + t.Errorf("GetSubjectType() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestGetId(t *testing.T) { + tests := []struct { + name string + data Data + expected string + }{ + { + name: "returns notification ID", + data: Data{ + Notification: data.NotificationData{ + Id: "123456789", + }, + }, + expected: "123456789", + }, + { + name: "returns empty string when not set", + data: Data{ + Notification: data.NotificationData{ + Id: "", + }, + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.data.GetId() + if result != tt.expected { + t.Errorf("GetId() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestGenerateActivityDescription(t *testing.T) { + tests := []struct { + name string + reason string + subjectType string + actor string + expected string + }{ + // comment reason + { + name: "comment on PR with actor", + reason: "comment", + subjectType: "PullRequest", + actor: "octocat", + expected: "@octocat commented on this pull request", + }, + { + name: "comment on Issue with actor", + reason: "comment", + subjectType: "Issue", + actor: "octocat", + expected: "@octocat commented on this issue", + }, + { + name: "comment on other type with actor", + reason: "comment", + subjectType: "Discussion", + actor: "octocat", + expected: "@octocat commented", + }, + { + name: "comment without actor", + reason: "comment", + subjectType: "PullRequest", + actor: "", + expected: "New comment", + }, + + // review_requested reason + { + name: "review requested with actor", + reason: "review_requested", + subjectType: "PullRequest", + actor: "reviewer", + expected: "@reviewer requested your review", + }, + { + name: "review requested without actor", + reason: "review_requested", + subjectType: "PullRequest", + actor: "", + expected: "Review requested", + }, + + // mention reason + { + name: "mention with actor", + reason: "mention", + subjectType: "Issue", + actor: "commenter", + expected: "@commenter mentioned you", + }, + { + name: "mention without actor", + reason: "mention", + subjectType: "Issue", + actor: "", + expected: "You were mentioned", + }, + + // author reason + { + name: "author reason", + reason: "author", + subjectType: "PullRequest", + actor: "someone", + expected: "Activity on your thread", + }, + + // assign reason + { + name: "assign reason", + reason: "assign", + subjectType: "Issue", + actor: "assigner", + expected: "You were assigned", + }, + + // state_change reason + { + name: "state change on PR", + reason: "state_change", + subjectType: "PullRequest", + actor: "", + expected: "Pull request state changed", + }, + { + name: "state change on Issue", + reason: "state_change", + subjectType: "Issue", + actor: "", + expected: "Issue state changed", + }, + { + name: "state change on other type", + reason: "state_change", + subjectType: "Discussion", + actor: "", + expected: "State changed", + }, + + // ci_activity reason + { + name: "CI activity", + reason: "ci_activity", + subjectType: "CheckSuite", + actor: "", + expected: "CI activity", + }, + + // subscribed reason + { + name: "subscribed PR with actor", + reason: "subscribed", + subjectType: "PullRequest", + actor: "contributor", + expected: "@contributor commented on this pull request", + }, + { + name: "subscribed Issue with actor", + reason: "subscribed", + subjectType: "Issue", + actor: "contributor", + expected: "@contributor commented on this issue", + }, + { + name: "subscribed other type with actor", + reason: "subscribed", + subjectType: "Discussion", + actor: "contributor", + expected: "Activity on subscribed thread", + }, + { + name: "subscribed without actor", + reason: "subscribed", + subjectType: "PullRequest", + actor: "", + expected: "Activity on subscribed thread", + }, + + // team_mention reason + { + name: "team mention", + reason: "team_mention", + subjectType: "Issue", + actor: "", + expected: "Your team was mentioned", + }, + + // security_alert reason + { + name: "security alert", + reason: "security_alert", + subjectType: "RepositoryVulnerabilityAlert", + actor: "", + expected: "Security vulnerability detected", + }, + + // unknown reason + { + name: "unknown reason with actor", + reason: "unknown_reason", + subjectType: "PullRequest", + actor: "someone", + expected: "@someone triggered this notification", + }, + { + name: "unknown reason without actor", + reason: "unknown_reason", + subjectType: "PullRequest", + actor: "", + expected: "", + }, + { + name: "empty reason without actor", + reason: "", + subjectType: "PullRequest", + actor: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GenerateActivityDescription(tt.reason, tt.subjectType, tt.actor) + if result != tt.expected { + t.Errorf("GenerateActivityDescription(%q, %q, %q) = %q, want %q", + tt.reason, tt.subjectType, tt.actor, result, tt.expected) + } + }) + } +} diff --git a/internal/tui/components/notificationrow/notificationrow.go b/internal/tui/components/notificationrow/notificationrow.go new file mode 100644 index 000000000..9f46eb9e7 --- /dev/null +++ b/internal/tui/components/notificationrow/notificationrow.go @@ -0,0 +1,235 @@ +package notificationrow + +import ( + "fmt" + "strings" + + "github.com/charmbracelet/lipgloss" + + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/table" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/utils" +) + +type Notification struct { + Ctx *context.ProgramContext + Data *Data +} + +func (n *Notification) ToTableRow() table.Row { + return table.Row{ + n.renderType(), + n.renderTitleBlock(), + n.renderActivity(), + n.renderUpdatedAt(), + } +} + +func (n *Notification) getTextStyle() lipgloss.Style { + return components.GetIssueTextStyle(n.Ctx) +} + +// getReadAwareStyle returns the text style for notifications. +// Read/unread status is indicated by the blue dot, not text color. +func (n *Notification) getReadAwareStyle() lipgloss.Style { + return n.getTextStyle() +} + +func (n *Notification) renderType() string { + style := lipgloss.NewStyle() + isUnread := n.Data.IsUnread() + + // Icons are always colored - only the blue dot indicates unread status + var icon string + switch n.Data.GetSubjectType() { + case "PullRequest": + // Use state-based icons/colors matching prrow.go + switch n.Data.SubjectState { + case "MERGED": + style = style.Foreground(n.Ctx.Styles.Colors.MergedPR) + icon = constants.MergedIcon + case "CLOSED": + style = style.Foreground(n.Ctx.Styles.Colors.ClosedPR) + icon = constants.ClosedIcon + default: // OPEN or unknown (not yet fetched) + if n.Data.IsDraft { + style = style.Foreground(n.Ctx.Theme.FaintText) + icon = constants.DraftIcon + } else { + style = style.Foreground(n.Ctx.Styles.Colors.OpenPR) + icon = constants.OpenIcon + } + } + case "Issue": + // Use state-based icons/colors matching issuerow.go + if n.Data.SubjectState == "CLOSED" { + icon = "" + style = style.Foreground(n.Ctx.Styles.Colors.ClosedPR) + } else { + icon = "" + style = style.Foreground(n.Ctx.Styles.Colors.OpenIssue) + } + case "Discussion": + icon = "" + style = style.Foreground(lipgloss.AdaptiveColor{Light: "#000000", Dark: "#ffffff"}) + case "Release": + icon = "" + style = style.Foreground(lipgloss.AdaptiveColor{Light: "#0969da", Dark: "#58a6ff"}) + case "Commit": + icon = "" + style = style.Foreground(n.Ctx.Theme.SecondaryText) + case "CheckSuite": + // Parse title to determine workflow status (similar to gitify approach) + title := strings.ToLower(n.Data.GetTitle()) + switch { + case strings.Contains(title, "failed"): + icon = constants.FailureIcon + style = style.Foreground(n.Ctx.Theme.ErrorText) + case strings.Contains(title, "succeeded"): + icon = constants.SuccessIcon + style = style.Foreground(n.Ctx.Theme.SuccessText) + case strings.Contains(title, "cancelled"), strings.Contains(title, "canceled"): + icon = constants.WorkflowRunIcon + style = style.Foreground(n.Ctx.Theme.FaintText) + case strings.Contains(title, "skipped"): + icon = constants.WorkflowRunIcon + style = style.Foreground(n.Ctx.Theme.FaintText) + default: + icon = constants.WorkflowRunIcon + style = style.Foreground(n.Ctx.Theme.WarningText) + } + case "RepositoryVulnerabilityAlert": + icon = constants.SecurityIcon + style = style.Foreground(n.Ctx.Theme.ErrorText) + default: + // Generic notification icon for unknown types + icon = constants.NotificationIcon + style = style.Foreground(n.Ctx.Theme.SecondaryText) + } + + // Use raw ANSI codes without reset to preserve parent background colors + iconPrefix := utils.GetStylePrefix(style) + + // Always render 3 lines to match the Title column (repo, title, activity) + // This ensures proper background color when row is selected + // Line 1: icon + // Line 2: blue dot (unread) or empty + // Line 3: empty + if isUnread { + dotPrefix := utils.GetStylePrefix(lipgloss.NewStyle().Foreground(lipgloss.Color("33"))) + return iconPrefix + icon + "\n" + dotPrefix + constants.DotIcon + "\n" + } + return iconPrefix + icon + "\n\n" +} + +// renderTitleBlock returns a 3-line block: +// Line 1: repo/name #number [bookmark icon if bookmarked] +// Line 2: Title (bold for unread) +// Line 3: Activity description +// Note: Truncation is handled dynamically by the table component based on actual column width +// Note: Uses raw ANSI codes without resets to preserve parent background colors +func (n *Notification) renderTitleBlock() string { + // Line 1: repo #number (secondary color, no ANSI reset to preserve background) + // Read/unread status is indicated by the blue dot, not text color + repoStyle := lipgloss.NewStyle().Foreground(n.Ctx.Theme.SecondaryText) + repoPrefix := utils.GetStylePrefix(repoStyle) + repo := n.Data.GetRepoNameWithOwner() + number := n.Data.GetNumber() + line1 := repo + if number > 0 { + line1 = fmt.Sprintf("%s #%d", repo, number) + } + // Add bookmark icon if bookmarked (using raw ANSI for warning color) + if data.GetBookmarkStore().IsBookmarked(n.Data.GetId()) { + bookmarkPrefix := utils.GetStylePrefix(lipgloss.NewStyle().Foreground(n.Ctx.Theme.WarningText)) + line1 = line1 + " " + bookmarkPrefix + "" + } + line1Rendered := repoPrefix + line1 + + // Line 2: Title (bold for unread) + titleStyle := n.getReadAwareStyle() + if n.Data.IsUnread() { + titleStyle = titleStyle.Bold(true) + } + titlePrefix := utils.GetStylePrefix(titleStyle) + title := n.Data.GetTitle() + line2Rendered := titlePrefix + title + + // Line 3: Activity description (no ANSI reset) + activityPrefix := utils.GetStylePrefix(lipgloss.NewStyle().Foreground(n.Ctx.Theme.FaintText)) + line3 := n.Data.ActivityDescription + if line3 == "" { + // Fallback to reason-based description + line3 = n.getReasonDescription() + } + line3Rendered := activityPrefix + line3 + + return line1Rendered + "\n" + line2Rendered + "\n" + line3Rendered +} + +// getReasonDescription returns a fallback description based on notification reason +func (n *Notification) getReasonDescription() string { + reason := n.Data.GetReason() + subjectType := n.Data.GetSubjectType() + + switch reason { + case "review_requested": + return "Review requested" + case "subscribed": + return "Activity on subscribed thread" + case "mention": + return "You were mentioned" + case "author": + return "Activity on your thread" + case "comment": + switch subjectType { + case "PullRequest": + return "New comment on pull request" + case "Issue": + return "New comment on issue" + } + return "New comment" + case "assign": + return "You were assigned" + case "state_change": + return "State changed" + case "ci_activity": + return "CI activity" + default: + return "" + } +} + +// renderActivity shows the new comments count with icon +// Returns 3 lines to match Title column for proper background highlighting +func (n *Notification) renderActivity() string { + if n.Data.NewCommentsCount <= 0 { + return "\n\n" + } + // Use raw ANSI foreground codes without reset to avoid breaking row background + // White foreground for count, green foreground for icon + white := "\x1b[97m" // Bright white + green := "\x1b[32m" // Green + return white + fmt.Sprintf("+%d ", n.Data.NewCommentsCount) + green + constants.CommentsIcon + "\n\n" +} + +// renderUpdatedAt returns the time since last update +// Returns 3 lines to match Title column for proper background highlighting +func (n *Notification) renderUpdatedAt() string { + timeFormat := n.Ctx.Config.Defaults.DateFormat + + updatedAtOutput := "" + if timeFormat == "" || timeFormat == "relative" { + // Use non-breaking space (U+00A0) to prevent wrap + updatedAtOutput = utils.TimeElapsed(n.Data.GetUpdatedAt()) + "\u00A0ago" + } else { + updatedAtOutput = n.Data.GetUpdatedAt().Format(timeFormat) + } + + // Use raw ANSI codes without reset to preserve parent background colors + stylePrefix := utils.GetStylePrefix(n.getReadAwareStyle()) + return stylePrefix + updatedAtOutput + "\n\n" +} diff --git a/internal/tui/components/notificationrow/notificationrow_test.go b/internal/tui/components/notificationrow/notificationrow_test.go new file mode 100644 index 000000000..9df287b73 --- /dev/null +++ b/internal/tui/components/notificationrow/notificationrow_test.go @@ -0,0 +1,377 @@ +package notificationrow + +import ( + "strings" + "testing" + + "github.com/dlvhdr/gh-dash/v4/internal/data" +) + +func TestGetReasonDescription(t *testing.T) { + tests := []struct { + name string + reason string + subjectType string + expected string + }{ + // review_requested + { + name: "review_requested", + reason: "review_requested", + subjectType: "PullRequest", + expected: "Review requested", + }, + // subscribed + { + name: "subscribed PR", + reason: "subscribed", + subjectType: "PullRequest", + expected: "Activity on subscribed thread", + }, + { + name: "subscribed Issue", + reason: "subscribed", + subjectType: "Issue", + expected: "Activity on subscribed thread", + }, + // mention + { + name: "mention", + reason: "mention", + subjectType: "Issue", + expected: "You were mentioned", + }, + // author + { + name: "author", + reason: "author", + subjectType: "PullRequest", + expected: "Activity on your thread", + }, + // comment + { + name: "comment on PR", + reason: "comment", + subjectType: "PullRequest", + expected: "New comment on pull request", + }, + { + name: "comment on Issue", + reason: "comment", + subjectType: "Issue", + expected: "New comment on issue", + }, + { + name: "comment on other type", + reason: "comment", + subjectType: "Discussion", + expected: "New comment", + }, + // assign + { + name: "assign", + reason: "assign", + subjectType: "Issue", + expected: "You were assigned", + }, + // state_change + { + name: "state_change PR", + reason: "state_change", + subjectType: "PullRequest", + expected: "State changed", + }, + // ci_activity + { + name: "ci_activity", + reason: "ci_activity", + subjectType: "CheckSuite", + expected: "CI activity", + }, + // unknown/empty + { + name: "unknown reason", + reason: "unknown_reason", + subjectType: "PullRequest", + expected: "", + }, + { + name: "empty reason", + reason: "", + subjectType: "PullRequest", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + n := &Notification{ + Data: &Data{ + Notification: data.NotificationData{ + Reason: tt.reason, + Subject: data.NotificationSubject{ + Type: tt.subjectType, + }, + }, + }, + } + result := n.getReasonDescription() + if result != tt.expected { + t.Errorf("getReasonDescription() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestParseCheckSuiteStatus(t *testing.T) { + // Test that CheckSuite status parsing from title works correctly + // This is tested indirectly through renderType, but we can verify + // the title parsing logic + + tests := []struct { + name string + title string + expectsFailed bool + expectsSuccess bool + expectsCancelled bool + expectsSkipped bool + }{ + { + name: "failed status", + title: "CI / build (push) failed", + expectsFailed: true, + }, + { + name: "succeeded status", + title: "CI / build (push) succeeded", + expectsSuccess: true, + }, + { + name: "cancelled status", + title: "CI / build (push) cancelled", + expectsCancelled: true, + }, + { + name: "canceled status (US spelling)", + title: "CI / build (push) canceled", + expectsCancelled: true, + }, + { + name: "skipped status", + title: "CI / build (push) skipped", + expectsSkipped: true, + }, + { + name: "unknown status", + title: "CI / build (push) running", + }, + { + name: "case insensitive - FAILED", + title: "CI / build (push) FAILED", + expectsFailed: true, + }, + { + name: "case insensitive - SUCCEEDED", + title: "CI / build (push) SUCCEEDED", + expectsSuccess: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + titleLower := strings.ToLower(tt.title) + + isFailed := strings.Contains(titleLower, "failed") + isSuccess := strings.Contains(titleLower, "succeeded") + isCancelled := strings.Contains(titleLower, "cancelled") || strings.Contains(titleLower, "canceled") + isSkipped := strings.Contains(titleLower, "skipped") + + if tt.expectsFailed && !isFailed { + t.Error("Expected failed to be detected") + } + if tt.expectsSuccess && !isSuccess { + t.Error("Expected succeeded to be detected") + } + if tt.expectsCancelled && !isCancelled { + t.Error("Expected cancelled/canceled to be detected") + } + if tt.expectsSkipped && !isSkipped { + t.Error("Expected skipped to be detected") + } + }) + } +} + +func TestRenderActivityOutput(t *testing.T) { + // Test renderActivity output format + tests := []struct { + name string + newCommentsCount int + expectsEmpty bool + expectsCount bool + }{ + { + name: "zero comments returns empty lines", + newCommentsCount: 0, + expectsEmpty: true, + }, + { + name: "negative comments returns empty lines", + newCommentsCount: -1, + expectsEmpty: true, + }, + { + name: "positive comments shows count", + newCommentsCount: 5, + expectsCount: true, + }, + { + name: "single comment shows count", + newCommentsCount: 1, + expectsCount: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + n := &Notification{ + Data: &Data{ + NewCommentsCount: tt.newCommentsCount, + }, + } + result := n.renderActivity() + + if tt.expectsEmpty { + // Should be just newlines (empty content with 3-line structure) + if result != "\n\n" { + t.Errorf("renderActivity() = %q, want empty lines", result) + } + } + if tt.expectsCount { + // Should contain the count + if !strings.Contains(result, "+") { + t.Errorf("renderActivity() = %q, should contain '+' for count", result) + } + } + }) + } +} + +func TestDataState(t *testing.T) { + // Test that SubjectState and IsDraft are properly used + tests := []struct { + name string + subjectState string + isDraft bool + subjectType string + }{ + { + name: "open PR", + subjectState: StateOpen, + isDraft: false, + subjectType: "PullRequest", + }, + { + name: "draft PR", + subjectState: StateOpen, + isDraft: true, + subjectType: "PullRequest", + }, + { + name: "merged PR", + subjectState: StateMerged, + isDraft: false, + subjectType: "PullRequest", + }, + { + name: "closed PR", + subjectState: StateClosed, + isDraft: false, + subjectType: "PullRequest", + }, + { + name: "open Issue", + subjectState: StateOpen, + isDraft: false, + subjectType: "Issue", + }, + { + name: "closed Issue", + subjectState: StateClosed, + isDraft: false, + subjectType: "Issue", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := Data{ + SubjectState: tt.subjectState, + IsDraft: tt.isDraft, + Notification: data.NotificationData{ + Subject: data.NotificationSubject{ + Type: tt.subjectType, + }, + }, + } + + // Verify the data is set correctly + if d.SubjectState != tt.subjectState { + t.Errorf("SubjectState = %q, want %q", d.SubjectState, tt.subjectState) + } + if d.IsDraft != tt.isDraft { + t.Errorf("IsDraft = %v, want %v", d.IsDraft, tt.isDraft) + } + if d.GetSubjectType() != tt.subjectType { + t.Errorf("GetSubjectType() = %q, want %q", d.GetSubjectType(), tt.subjectType) + } + }) + } +} + +func TestActivityDescriptionFallback(t *testing.T) { + // Test that ActivityDescription is used when set, else falls back to reason + tests := []struct { + name string + activityDescription string + reason string + subjectType string + expectContains string + }{ + { + name: "uses ActivityDescription when set", + activityDescription: "@user commented on this PR", + reason: "comment", + subjectType: "PullRequest", + expectContains: "@user commented", + }, + { + name: "falls back to reason when ActivityDescription empty", + activityDescription: "", + reason: "review_requested", + subjectType: "PullRequest", + expectContains: "Review requested", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := Data{ + ActivityDescription: tt.activityDescription, + Notification: data.NotificationData{ + Reason: tt.reason, + Subject: data.NotificationSubject{ + Type: tt.subjectType, + }, + }, + } + + // When ActivityDescription is set, it should be used + if tt.activityDescription != "" { + if d.ActivityDescription != tt.activityDescription { + t.Errorf("ActivityDescription = %q, want %q", d.ActivityDescription, tt.activityDescription) + } + } + }) + } +} diff --git a/internal/tui/components/notificationssection/DESIGN.md b/internal/tui/components/notificationssection/DESIGN.md new file mode 100644 index 000000000..1868d2da8 --- /dev/null +++ b/internal/tui/components/notificationssection/DESIGN.md @@ -0,0 +1,393 @@ +# Notifications Feature — Implementation & Design Summary + +## Overview + +The notifications feature adds a new view to gh-dash that displays GitHub notifications, allowing users to triage their inbox directly from the terminal. The implementation follows the existing patterns established by the PR and Issue views. + +## Architecture + +### File Structure + +``` +internal/ +├── data/ +│ ├── notificationapi.go # GitHub API interactions for notifications +│ └── bookmarks.go # Local bookmark storage (singleton) +├── tui/ +│ ├── keys/ +│ │ └── notificationKeys.go # Key bindings specific to notifications +│ └── components/ +│ ├── notificationrow/ +│ │ ├── data.go # Data model implementing RowData interface +│ │ ├── data_test.go # Tests for data accessors +│ │ ├── notificationrow.go # Row rendering for the table +│ │ └── notificationrow_test.go # Tests for rendering logic +│ ├── notificationssection/ +│ │ ├── notificationssection.go # Main section component +│ │ ├── commands.go # Tea commands (mark done, mark read, diff, checkout, etc.) +│ │ ├── commands_test.go # Tests for command functions +│ │ └── filters_test.go # Tests for filter parsing +│ └── notificationview/ +│ └── notificationview.go # Detail view in sidebar +``` + +### Key Design Decisions + +#### 1. Explicit View Action for Notifications + +Unlike PRs and Issues which auto-fetch content when selected, notifications require an explicit action (Enter key) to view content. This design choice exists because: + +- Viewing a notification marks it as read (GitHub API behavior) +- Users should consciously decide when to mark something as read +- Prevents accidental "read" marking when just browsing the list + +When a notification is selected but not yet viewed, a prompt is displayed in the Preview pane: + +``` + Press [Enter] to view the PR + (Note: this will mark it as read) + + Other Actions + + [D] mark as done + [m] mark as read + [u] unsubscribe + [b] toggle bookmark + [t] toggle filtering + [S] sort by repo + [o] open in browser + [Enter] view +``` + +- Keys are displayed with a background highlight +- Actions are displayed in green (success color) +- The note about marking as read appears for all notification types +- For PR/Issue types: "Press Enter to view the PR/Issue" +- For other notification types (Discussion, Release, etc.): "Press Enter to open in browser" + +#### 2. Notification Data Flow + +``` +GitHub REST API + │ + ▼ +notificationapi.go (FetchNotifications) + │ + ▼ +notificationssection.go (stores []notificationrow.Data) + │ + ├──▶ notificationrow.go (renders table rows) + │ + └──▶ notificationview.go (renders sidebar detail) + OR + renderNotificationPrompt() (shows action prompt) +``` + +#### 3. Comment Count Tracking + +For PR and Issue notifications, the system fetches additional data to show new comment counts: + +- Compares `LastReadAt` timestamp with comment timestamps +- Displays count of comments made since user last read the notification +- Scrolls to the latest comment when viewing PR/Issue notifications + +#### 4. Actor Display + +For PR and Issue notifications, the username of the person who triggered the notification is displayed: + +- Fetches the author from `latest_comment_url` (the comment that triggered the notification) +- Falls back to the PR/Issue author for new items without comments +- Helps identify spam without needing to open the notification +- Fetched alongside comment counts (no additional latency) +- Color is configurable via `theme.colors.text.actor` (defaults to secondary text color) + +#### 5. Three-Line Row Layout + +Each notification row displays three lines of information: + +``` +repo/name #123 🔖 +5💬 2d ago +Title of the notification (bold if unread) +@username commented on this pull request +``` + +(The 🔖 bookmark icon only appears if the notification is bookmarked) + +- **Line 1:** Repository name with issue/PR number, bookmark icon if bookmarked (SecondaryText color; bookmark icon in WarningText color) +- **Line 2:** Notification title (PrimaryText, bold for unread notifications) +- **Line 3:** Activity description (FaintText color, generated from reason, type, and actor) + +The three lines use distinct colors to create visual hierarchy: line 1 is secondary, line 2 is primary/bold, and line 3 is faint. + +**Unread indicator:** A blue dot is displayed below the notification type icon for unread notifications. This is the sole indicator of read/unread status—text is not dimmed for read notifications. + +Activity descriptions are generated based on the notification reason: +- `comment`: "@username commented on this pull request/issue" +- `review_requested`: "@username requested your review" or "Review requested" +- `mention`: "@username mentioned you" +- `author`: "Activity on your thread" +- `assign`: "You were assigned" +- `state_change`: "Pull request/Issue state changed" +- `ci_activity`: "CI activity" +- `subscribed`: "@username commented on this pull request/issue" + +#### 6. Title Sanitization + +Notification titles from GitHub's API may contain control characters (e.g., trailing `\r`) that corrupt terminal rendering. The `GetTitle()` method sanitizes titles by: +- Removing carriage return characters (`\r`) +- Replacing newlines (`\n`) with spaces +- Trimming leading/trailing whitespace + +#### 7. Multi-Line Row Background + +The table component applies cell styling to each line individually in multi-line content. This ensures the background color (for selected rows) extends properly across the entire cell. + +To preserve parent background colors, row content uses raw ANSI escape codes for foreground styling without trailing reset sequences. The `utils.GetStylePrefix()` helper extracts ANSI codes from lipgloss styles while stripping the reset, preventing internal resets from breaking the cell's background color. + +Title truncation is handled dynamically by the table component based on actual column width, with ellipsis added when content is truncated. This allows titles to adjust when the sidebar is shown/hidden. + +#### 8. Bookmark and Done Systems + +Both bookmarks and "done" status are tracked locally because GitHub's API doesn't provide these features. They share a common `NotificationIDStore` implementation in `data/bookmarks.go`: + +```go +type NotificationIDStore struct { + ids map[string]bool + filePath string + // ... mutex, name for logging +} +``` + +**Bookmarks** allow users to keep notifications visible even after marking them as read: + +- Stored in `~/.local/state/gh-dash/bookmarks.json` +- Accessed via `data.GetBookmarkStore()` singleton +- Bookmarked notifications appear in the default inbox view even when read +- Bookmarked notifications are styled as read (faint text) but show a bookmark indicator +- When user explicitly searches `is:unread`, bookmarked+read items are excluded + +**Done tracking** is necessary because GitHub's "mark as done" API (`DELETE /notifications/threads/{id}`) doesn't actually delete notifications — they still appear in API responses with `all=true`. Without local tracking, done notifications would reappear when filtering to `is:read` or `is:all`: + +- Stored in `~/.local/state/gh-dash/done.json` +- Accessed via `data.GetDoneStore()` singleton +- When marking a notification as done, its ID is persisted to the store +- Done notifications are filtered out during fetch, regardless of API response +- Persists across sessions and application restarts + +**Pagination with local filtering**: Because done notifications are filtered out locally after fetching from the API, a single page of results may yield very few visible notifications. To handle this, the fetch logic automatically requests additional pages from the API until the requested limit is reached or all pages are exhausted. This ensures users see a full page of results even when many notifications have been marked as done. + +#### 9. Unsubscribe + +The unsubscribe feature allows users to stop receiving notifications for a thread: + +- Uses GitHub's `DELETE /notifications/threads/{id}/subscription` API +- Removes the subscription without marking the notification as done +- Useful for threads that are no longer relevant but shouldn't be deleted + +#### 10. State Management + +Notification state (read/unread, done) is tracked both: +- Locally in the `notificationrow.Data` struct for immediate UI updates +- Remotely via GitHub API calls for persistence + +The `UpdateNotificationMsg` and `UpdateNotificationReadStateMsg` types propagate state changes through the Bubble Tea update cycle. + +**Session persistence for read notifications:** When a notification is marked as read (via `m` key or by viewing it), its ID is tracked in `sessionMarkedRead`. These notifications remain visible in the inbox even during automatic refreshes (e.g., when the terminal regains focus). They are only removed when: +- The user performs a manual refresh (Refresh key) +- The user quits and restarts the application +- The notification is explicitly marked as done + +#### 12. Command Architecture + +Notification commands are organized in `commands.go`, following the pattern established by `prssection` (which has `checkout.go`, `diff.go`). Commands fall into two categories: + +**Section methods** — Commands that operate on section state and are invoked via key handling in the section's `Update` method: +- `markAsDone()` — Marks the current notification as done +- `markAllAsDone()` — Marks all visible notifications as done +- `markAsRead()` — Marks the current notification as read +- `markAllAsRead()` — Marks all notifications as read +- `unsubscribe()` — Unsubscribes from the current thread +- `openInBrowser()` — Marks as read and opens in browser + +**Standalone functions** — Commands that require data from outside the section (e.g., the PR shown in the sidebar). These are called from `ui.go` with the necessary parameters: +- `DiffPR(ctx, prNumber, repoName)` — Opens a diff view for a PR +- `CheckoutPR(ctx, prNumber, repoName)` — Checks out a PR branch locally + +This split exists because diff and checkout operate on the PR/Issue content shown in the `notificationView` sidebar, not the notification row itself. When viewing a PR notification, the sidebar displays the full PR details, and diff/checkout actions use that data. The section doesn't have access to this enriched data, so `ui.go` extracts the PR details from `notificationView` and passes them to the standalone functions. + +Key events are routed through `ui.go`, which either: +1. Passes them to the section via `updateSection()` for section methods +2. Handles them directly and calls standalone functions with the required data + +### Interface Compliance + +`notificationrow.Data` implements the `data.RowData` interface: + +```go +type RowData interface { + GetRepoNameWithOwner() string + GetNumber() int + GetTitle() string + GetUrl() string +} +``` + +- `GetNumber()` extracts the PR/Issue number from the subject URL +- `GetUrl()` constructs the GitHub web URL from the API URL + +### Styling Consistency + +Common styling functions were extracted to `common/styles.go`: + +- `RenderPreviewHeader()` — renders the repo/type header line with background +- `RenderPreviewTitle()` — renders the title block with background highlight + +These are used by PR view, Issue view, notification view, and notification prompt for consistent appearance. + +### Table Column Alignment + +The table component was extended to support per-column alignment via an `Align` property on the `Column` struct. This allows the comment count column to be right-aligned. + +## Key Bindings + +| Key | Action | +|-----|--------| +| D | Mark as done (removes from inbox) | +| Alt+d | Mark all as done | +| m | Mark as read | +| M | Mark all as read | +| u | Unsubscribe from thread | +| b | Toggle bookmark | +| t | Toggle smart filtering (filter to current repo) | +| y | Copy PR/Issue number | +| Y | Copy URL | +| S | Sort by repository | +| o | Open in browser | +| Enter | View notification (fetches content, marks as read) | + +### PR/Issue Keybindings in Notifications View + +When viewing a PR notification in the preview pane, all PR-specific keybindings become available: + +| Key | Action | +|-----|--------| +| v | Approve PR | +| a | Assign | +| A | Unassign | +| c | Comment | +| d | View diff | +| C/Space | Checkout branch | +| x | Close PR | +| X | Reopen PR | +| W | Mark ready for review | +| m | Merge PR | +| u | Update from base branch | +| w | Watch checks | +| [ | Previous sidebar tab | +| ] | Next sidebar tab | +| e | Expand description | + +Similarly, when viewing an Issue notification, Issue-specific keybindings are available: + +| Key | Action | +|-----|--------| +| L | Add/remove labels | +| a | Assign | +| A | Unassign | +| c | Comment | +| x | Close issue | +| X | Reopen issue | + +The `?` help display dynamically updates to show the applicable keybindings based on what type of notification content is being viewed. + +## Configuration + +### Search Section + +Like PRs and Issues, the Notifications view includes a search section (indicated by a magnifying glass icon 🔍) as the first tab. This serves as a scratchpad for one-off searches without modifying your configured sections. + +- Default filter: `archived:false` +- Respects `smartFilteringAtLaunch`: when enabled and running from a git repository, the search automatically scopes to that repo +- Use the `/` key to focus the search bar and enter custom queries +- Supports all notification filters: `is:unread`, `is:read`, `repo:owner/name`, `reason:*` + +### Notification Sections + +Notifications support multiple configurable sections, similar to PRs and Issues. Each section appears as a tab and filters notifications by reason: + +```yaml +notificationsSections: + - title: All + filters: "" + - title: Created + filters: "reason:author" + - title: Participating + filters: "reason:participating" + - title: Mentioned + filters: "reason:mention" + - title: Review Requested + filters: "reason:review-requested" + - title: Assigned + filters: "reason:assign" + - title: Subscribed + filters: "reason:subscribed" + - title: Team Mentioned + filters: "reason:team-mention" +``` + +These are the default sections. Users can customize by defining their own `notificationsSections` in `config.yml`. + +#### Reason Filters + +The `reason:` filter matches GitHub's notification reason field: + +| Filter | Description | +|--------|-------------| +| `reason:author` | Activity on threads you created | +| `reason:comment` | Someone commented on a thread you're subscribed to | +| `reason:mention` | You were @mentioned | +| `reason:review-requested` | Your review was requested on a PR | +| `reason:assign` | You were assigned | +| `reason:subscribed` | Activity on threads you're watching | +| `reason:team-mention` | Your team was @mentioned | +| `reason:state-change` | Thread state changed (merged, closed, etc.) | +| `reason:ci-activity` | CI workflow activity | +| `reason:participating` | Meta-filter: expands to author, comment, mention, review-requested, assign, state-change | + +Reason filters are applied client-side after fetching from GitHub's API. + +### Fetch Limit + +The initial fetch limit is controlled by `defaults.notificationsLimit` (default: 20, matching PRs and Issues). Additional notifications are fetched automatically as the user scrolls through the list. + +```yaml +defaults: + notificationsLimit: 20 +``` + +### Smart Filtering + +Notifications respect the global `smartFilteringAtLaunch` setting (enabled by default). When enabled and running from within a git repository, notifications are automatically scoped to that repository. The search bar displays `repo:owner/name` to indicate this filtering. + +Users can: +- Press `t` to toggle filtering on/off for the current session +- Set `smartFilteringAtLaunch: false` in config to disable this behavior globally + +#### 11. CheckSuite URL Resolution + +GitHub's API returns `subject.url=null` for CheckSuite notifications, making it impossible to directly link to the specific workflow run. To work around this: + +1. Initially, CheckSuite notifications link to the repository's `/actions` page as a fallback +2. Asynchronously, we fetch recent workflow runs from `/repos/{owner}/{repo}/actions/runs` +3. We find the workflow run closest in time to the notification's `updated_at` timestamp +4. Once resolved, the notification's URL is updated to point to the specific workflow run + +This async resolution uses the existing `UpdateNotificationUrlMsg` message type, following the same pattern as async comment count fetching for PRs and Issues. + +## Limitations + +- **Mark as Unread**: GitHub's REST API does not support marking notifications as unread, so this feature is not available. Bookmarks provide a workaround by keeping items visible in the inbox. +- **Discussion/Release Content**: Only PR and Issue notifications can display detailed content in the sidebar; other types open directly in the browser. +- **Local State Persistence**: Bookmarks and done status are stored locally (`~/.local/state/gh-dash/`) and are not synced across machines or with GitHub. +- **Done Notifications in API**: GitHub's "mark as done" doesn't delete notifications — they still appear in API responses with `all=true`. We track done IDs locally to filter them out. +- **Server-Side Reason Filtering**: GitHub's notification API does not support filtering by reason on the server side. Reason filters are applied client-side after fetching notifications, which means all notifications are fetched before filtering. diff --git a/internal/tui/components/notificationssection/commands.go b/internal/tui/components/notificationssection/commands.go new file mode 100644 index 000000000..0fea7a89f --- /dev/null +++ b/internal/tui/components/notificationssection/commands.go @@ -0,0 +1,288 @@ +package notificationssection + +import ( + "errors" + "fmt" + "os" + "os/exec" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/cli/go-gh/v2/pkg/browser" + + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/common" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" +) + +func (m *Model) markAsDone() tea.Cmd { + notification := m.GetCurrNotification() + if notification == nil { + return nil + } + + notificationId := notification.GetId() + taskId := fmt.Sprintf("notification_done_%s", notificationId) + task := context.Task{ + Id: taskId, + StartText: "Marking notification as done", + FinishedText: "Notification marked as done", + State: context.TaskStart, + Error: nil, + } + startCmd := m.Ctx.StartTask(task) + return tea.Batch(startCmd, func() tea.Msg { + err := data.MarkNotificationDone(notificationId) + if err == nil { + // Persist to done store so it stays hidden across sessions + data.GetDoneStore().MarkDone(notificationId) + } + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: err, + Msg: UpdateNotificationMsg{ + Id: notificationId, + IsRemoved: err == nil, + }, + } + }) +} + +// markAllAsDone marks all currently visible notifications in this section as done. +// "All" refers to the notifications currently loaded in m.Notifications, not all +// notifications on GitHub. +func (m *Model) markAllAsDone() tea.Cmd { + if len(m.Notifications) == 0 { + return nil + } + + count := len(m.Notifications) + taskId := "notification_done_all" + task := context.Task{ + Id: taskId, + StartText: fmt.Sprintf("Marking %d notifications as done", count), + FinishedText: fmt.Sprintf("%d notifications marked as done", count), + State: context.TaskStart, + Error: nil, + } + + notificationIds := make([]string, 0, count) + for _, n := range m.Notifications { + notificationIds = append(notificationIds, n.GetId()) + } + + startCmd := m.Ctx.StartTask(task) + return tea.Batch(startCmd, func() tea.Msg { + // Mark each notification as done (delete it) + doneStore := data.GetDoneStore() + var lastErr error + for _, id := range notificationIds { + if err := data.MarkNotificationDone(id); err != nil { + lastErr = err + } else { + // Persist to done store so it stays hidden across sessions + doneStore.MarkDone(id) + } + } + + if lastErr != nil { + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: lastErr, + } + } + + // Clear all notifications after marking as done + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: nil, + Msg: ClearAllNotificationsMsg{}, + } + }) +} + +func (m *Model) markAllAsRead() tea.Cmd { + taskId := "notification_read_all" + task := context.Task{ + Id: taskId, + StartText: "Marking all notifications as read", + FinishedText: "All notifications marked as read", + State: context.TaskStart, + Error: nil, + } + startCmd := m.Ctx.StartTask(task) + return tea.Batch(startCmd, func() tea.Msg { + err := data.MarkAllNotificationsRead() + if err != nil { + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: err, + } + } + + // Update all notifications to read state + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: nil, + Msg: MarkAllAsReadMsg{}, + } + }) +} + +type ( + // RefetchNotificationsMsg signals that notifications should be refetched from the API + RefetchNotificationsMsg struct{} + // ClearAllNotificationsMsg signals that all notifications should be removed from the local list + // This is sent after successfully marking all notifications as done + ClearAllNotificationsMsg struct{} + // MarkAllAsReadMsg signals that all notifications should be updated to read state in the UI + // This is sent after successfully calling the mark-all-read API + MarkAllAsReadMsg struct{} +) + +func (m *Model) markAsRead() tea.Cmd { + notification := m.GetCurrNotification() + if notification == nil { + return nil + } + + notificationId := notification.GetId() + taskId := fmt.Sprintf("notification_read_%s", notificationId) + task := context.Task{ + Id: taskId, + StartText: "Marking notification as read", + FinishedText: "Notification marked as read", + State: context.TaskStart, + Error: nil, + } + startCmd := m.Ctx.StartTask(task) + return tea.Batch(startCmd, func() tea.Msg { + err := data.MarkNotificationRead(notificationId) + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: err, + Msg: UpdateNotificationReadStateMsg{ + Id: notificationId, + Unread: false, + }, + } + }) +} + +func (m *Model) unsubscribe() tea.Cmd { + notification := m.GetCurrNotification() + if notification == nil { + return nil + } + + notificationId := notification.GetId() + taskId := fmt.Sprintf("notification_unsubscribe_%s", notificationId) + task := context.Task{ + Id: taskId, + StartText: "Unsubscribing from thread", + FinishedText: "Unsubscribed from thread", + State: context.TaskStart, + Error: nil, + } + startCmd := m.Ctx.StartTask(task) + return tea.Batch(startCmd, func() tea.Msg { + err := data.UnsubscribeFromThread(notificationId) + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: SectionType, + TaskId: taskId, + Err: err, + Msg: UnsubscribedMsg{ + Id: notificationId, + }, + } + }) +} + +// UnsubscribedMsg is sent when a notification thread is unsubscribed +type UnsubscribedMsg struct { + Id string +} + +// UpdateNotificationReadStateMsg is sent when a notification's read state changes +type UpdateNotificationReadStateMsg struct { + Id string + Unread bool +} + +// openInBrowser marks the current notification as read and opens it in the browser +func (m *Model) openInBrowser() tea.Cmd { + notification := m.GetCurrNotification() + if notification == nil { + return nil + } + + notificationId := notification.GetId() + notificationUrl := notification.GetUrl() + + return tea.Batch( + func() tea.Msg { + _ = data.MarkNotificationRead(notificationId) + return UpdateNotificationReadStateMsg{ + Id: notificationId, + Unread: false, + } + }, + func() tea.Msg { + b := browser.New("", os.Stdout, os.Stdin) + err := b.Browse(notificationUrl) + if err != nil { + return constants.ErrMsg{Err: err} + } + return nil + }, + ) +} + +// CheckoutPR checks out a PR. This is a standalone function that can be called +// from ui.go with the PR details from the notification view. +func CheckoutPR(ctx *context.ProgramContext, prNumber int, repoName string) (tea.Cmd, error) { + repoPath, ok := common.GetRepoLocalPath(repoName, ctx.Config.RepoPaths) + if !ok { + return nil, errors.New("local path to repo not specified, set one in your config.yml under repoPaths") + } + + taskId := fmt.Sprintf("checkout_%d", prNumber) + task := context.Task{ + Id: taskId, + StartText: fmt.Sprintf("Checking out PR #%d", prNumber), + FinishedText: fmt.Sprintf("PR #%d has been checked out at %s", prNumber, repoPath), + State: context.TaskStart, + Error: nil, + } + startCmd := ctx.StartTask(task) + return tea.Batch(startCmd, func() tea.Msg { + c := exec.Command( + "gh", + "pr", + "checkout", + fmt.Sprint(prNumber), + ) + userHomeDir, _ := os.UserHomeDir() + if strings.HasPrefix(repoPath, "~") { + repoPath = strings.Replace(repoPath, "~", userHomeDir, 1) + } + + c.Dir = repoPath + err := c.Run() + return constants.TaskFinishedMsg{TaskId: taskId, Err: err} + }), nil +} diff --git a/internal/tui/components/notificationssection/commands_test.go b/internal/tui/components/notificationssection/commands_test.go new file mode 100644 index 000000000..52e665efc --- /dev/null +++ b/internal/tui/components/notificationssection/commands_test.go @@ -0,0 +1,109 @@ +package notificationssection + +import ( + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/dlvhdr/gh-dash/v4/internal/config" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" +) + +// noopStartTask is a stub that returns nil for testing +func noopStartTask(task context.Task) tea.Cmd { + return nil +} + +func TestCheckoutPR(t *testing.T) { + tests := []struct { + name string + prNumber int + repoName string + repoPaths map[string]string + wantErr bool + wantNil bool + }{ + { + name: "returns error when repo path not configured", + prNumber: 123, + repoName: "owner/repo", + repoPaths: map[string]string{}, + wantErr: true, + wantNil: true, + }, + { + name: "returns command when repo path is configured", + prNumber: 123, + repoName: "owner/repo", + repoPaths: map[string]string{ + "owner/repo": "/path/to/repo", + }, + wantErr: false, + wantNil: false, + }, + { + name: "returns command with tilde path", + prNumber: 456, + repoName: "my-org/my-repo", + repoPaths: map[string]string{ + "my-org/my-repo": "~/projects/my-repo", + }, + wantErr: false, + wantNil: false, + }, + { + name: "returns error for unconfigured repo even with other repos configured", + prNumber: 789, + repoName: "other/repo", + repoPaths: map[string]string{"owner/repo": "/path/to/repo"}, + wantErr: true, + wantNil: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := &context.ProgramContext{ + Config: &config.Config{ + RepoPaths: tt.repoPaths, + }, + StartTask: noopStartTask, + } + + cmd, err := CheckoutPR(ctx, tt.prNumber, tt.repoName) + + if tt.wantErr && err == nil { + t.Errorf("CheckoutPR() error = nil, want error") + } + if !tt.wantErr && err != nil { + t.Errorf("CheckoutPR() error = %v, want nil", err) + } + if tt.wantNil && cmd != nil { + t.Errorf("CheckoutPR() returned non-nil cmd, want nil") + } + if !tt.wantNil && cmd == nil { + t.Errorf("CheckoutPR() returned nil cmd, want non-nil") + } + }) + } +} + +func TestCheckoutPRErrorMessage(t *testing.T) { + ctx := &context.ProgramContext{ + Config: &config.Config{ + RepoPaths: map[string]string{}, + }, + StartTask: noopStartTask, + } + + _, err := CheckoutPR(ctx, 123, "owner/repo") + + if err == nil { + t.Fatal("CheckoutPR() expected error, got nil") + } + + expectedMsg := "local path to repo not specified, set one in your config.yml under repoPaths" + if err.Error() != expectedMsg { + t.Errorf("CheckoutPR() error = %q, want %q", err.Error(), expectedMsg) + } +} diff --git a/internal/tui/components/notificationssection/filters_test.go b/internal/tui/components/notificationssection/filters_test.go new file mode 100644 index 000000000..d55c54226 --- /dev/null +++ b/internal/tui/components/notificationssection/filters_test.go @@ -0,0 +1,479 @@ +package notificationssection + +import ( + "testing" + + "github.com/dlvhdr/gh-dash/v4/internal/data" +) + +func TestParseNotificationFilters(t *testing.T) { + tests := []struct { + name string + search string + wantReadState data.NotificationReadState + wantIsDone bool + wantExplicitUnread bool + wantIncludeBookmarked bool + wantRepoCount int + }{ + { + name: "empty search defaults to unread with bookmarks", + search: "", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, + wantRepoCount: 0, + }, + { + name: "explicit is:unread excludes bookmarks", + search: "is:unread", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: true, + wantIncludeBookmarked: false, + wantRepoCount: 0, + }, + { + name: "is:read excludes bookmarks", + search: "is:read", + wantReadState: data.NotificationStateRead, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: false, + wantRepoCount: 0, + }, + { + name: "is:all excludes bookmarks", + search: "is:all", + wantReadState: data.NotificationStateAll, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: false, + wantRepoCount: 0, + }, + { + name: "is:done sets IsDone flag", + search: "is:done", + wantReadState: data.NotificationStateUnread, + wantIsDone: true, + wantExplicitUnread: false, + wantIncludeBookmarked: true, + wantRepoCount: 0, + }, + { + name: "is:unread and is:read together becomes is:all", + search: "is:unread is:read", + wantReadState: data.NotificationStateAll, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: false, + wantRepoCount: 0, + }, + { + name: "repo filter is extracted", + search: "repo:owner/repo", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, + wantRepoCount: 1, + }, + { + name: "multiple repo filters", + search: "repo:owner/repo1 repo:owner/repo2", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, + wantRepoCount: 2, + }, + { + name: "combined filters", + search: "repo:owner/repo is:unread", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: true, + wantIncludeBookmarked: false, + wantRepoCount: 1, + }, + { + name: "random text preserves defaults", + search: "some random search text", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, + wantRepoCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filters := parseNotificationFilters(tt.search) + + if filters.ReadState != tt.wantReadState { + t.Errorf("ReadState = %v, want %v", filters.ReadState, tt.wantReadState) + } + if filters.IsDone != tt.wantIsDone { + t.Errorf("IsDone = %v, want %v", filters.IsDone, tt.wantIsDone) + } + if filters.ExplicitUnread != tt.wantExplicitUnread { + t.Errorf("ExplicitUnread = %v, want %v", filters.ExplicitUnread, tt.wantExplicitUnread) + } + if filters.IncludeBookmarked != tt.wantIncludeBookmarked { + t.Errorf("IncludeBookmarked = %v, want %v", filters.IncludeBookmarked, tt.wantIncludeBookmarked) + } + if len(filters.RepoFilters) != tt.wantRepoCount { + t.Errorf("RepoFilters count = %d, want %d", len(filters.RepoFilters), tt.wantRepoCount) + } + }) + } +} + +func TestParseReasonFilters(t *testing.T) { + tests := []struct { + name string + search string + expected []string + }{ + { + name: "no reason filter", + search: "is:unread", + expected: []string{}, + }, + { + name: "single reason filter", + search: "reason:author", + expected: []string{data.ReasonAuthor}, + }, + { + name: "multiple reason filters", + search: "reason:author reason:mention", + expected: []string{data.ReasonAuthor, data.ReasonMention}, + }, + { + name: "reason:participating expands to multiple reasons", + search: "reason:participating", + expected: []string{ + data.ReasonAuthor, + data.ReasonComment, + data.ReasonMention, + data.ReasonReviewRequested, + data.ReasonAssign, + data.ReasonStateChange, + }, + }, + { + name: "reason:review-requested normalizes to review_requested", + search: "reason:review-requested", + expected: []string{data.ReasonReviewRequested}, + }, + { + name: "reason:team-mention normalizes to team_mention", + search: "reason:team-mention", + expected: []string{data.ReasonTeamMention}, + }, + { + name: "reason:ci-activity normalizes to ci_activity", + search: "reason:ci-activity", + expected: []string{data.ReasonCIActivity}, + }, + { + name: "reason:security-alert normalizes to security_alert", + search: "reason:security-alert", + expected: []string{data.ReasonSecurityAlert}, + }, + { + name: "reason:state-change normalizes to state_change", + search: "reason:state-change", + expected: []string{data.ReasonStateChange}, + }, + { + name: "reason filter mixed with other text", + search: "is:unread reason:author some text", + expected: []string{data.ReasonAuthor}, + }, + { + name: "direct reason values pass through", + search: "reason:subscribed reason:comment", + expected: []string{data.ReasonSubscribed, data.ReasonComment}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseReasonFilters(tt.search) + + if len(result) != len(tt.expected) { + t.Errorf("parseReasonFilters(%q) returned %d items, want %d", tt.search, len(result), len(tt.expected)) + t.Errorf("got: %v", result) + t.Errorf("want: %v", tt.expected) + return + } + + for i, want := range tt.expected { + if result[i] != want { + t.Errorf("parseReasonFilters(%q)[%d] = %q, want %q", tt.search, i, result[i], want) + } + } + }) + } +} + +func TestParseNotificationFiltersWithReasons(t *testing.T) { + tests := []struct { + name string + search string + wantReasonCount int + }{ + { + name: "no reason filter", + search: "", + wantReasonCount: 0, + }, + { + name: "single reason filter", + search: "reason:author", + wantReasonCount: 1, + }, + { + name: "reason:participating expands to 6 reasons", + search: "reason:participating", + wantReasonCount: 6, + }, + { + name: "combined with other filters", + search: "is:unread repo:owner/repo reason:mention", + wantReasonCount: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filters := parseNotificationFilters(tt.search) + if len(filters.ReasonFilters) != tt.wantReasonCount { + t.Errorf("ReasonFilters count = %d, want %d", len(filters.ReasonFilters), tt.wantReasonCount) + } + }) + } +} + +func TestParseRepoFilters(t *testing.T) { + tests := []struct { + name string + search string + expected []string + }{ + { + name: "no repo filter", + search: "is:unread", + expected: []string{}, + }, + { + name: "single repo filter", + search: "repo:owner/repo", + expected: []string{"owner/repo"}, + }, + { + name: "multiple repo filters", + search: "repo:owner/repo1 repo:other/repo2", + expected: []string{"owner/repo1", "other/repo2"}, + }, + { + name: "repo filter with hyphen", + search: "repo:my-org/my-repo", + expected: []string{"my-org/my-repo"}, + }, + { + name: "repo filter mixed with other text", + search: "is:unread repo:owner/repo some text", + expected: []string{"owner/repo"}, + }, + { + name: "repo filter with underscore", + search: "repo:my_org/my_repo", + expected: []string{"my_org/my_repo"}, + }, + { + name: "repo filter with numbers", + search: "repo:org123/repo456", + expected: []string{"org123/repo456"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseRepoFilters(tt.search) + + if len(result) != len(tt.expected) { + t.Errorf("parseRepoFilters(%q) returned %d items, want %d", tt.search, len(result), len(tt.expected)) + return + } + + for i, want := range tt.expected { + if result[i] != want { + t.Errorf("parseRepoFilters(%q)[%d] = %q, want %q", tt.search, i, result[i], want) + } + } + }) + } +} + +func TestParseNotificationFiltersEdgeCases(t *testing.T) { + tests := []struct { + name string + search string + wantReadState data.NotificationReadState + wantIsDone bool + wantExplicitUnread bool + wantIncludeBookmarked bool + }{ + { + name: "whitespace only preserves defaults", + search: " ", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, + }, + { + name: "mixed case is:UNREAD not recognized (case sensitive)", + search: "is:UNREAD", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: false, // Not recognized due to case + wantIncludeBookmarked: true, // Default when not recognized + }, + { + name: "mixed case is:Read not recognized (case sensitive)", + search: "is:Read", + wantReadState: data.NotificationStateUnread, // Default when not recognized + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, // Default when not recognized + }, + { + name: "mixed case is:ALL not recognized (case sensitive)", + search: "is:ALL", + wantReadState: data.NotificationStateUnread, // Default when not recognized + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: true, // Default when not recognized + }, + { + name: "mixed case is:Done not recognized (case sensitive)", + search: "is:Done", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, // Not recognized due to case + wantExplicitUnread: false, + wantIncludeBookmarked: true, + }, + { + name: "multiple spaces between filters", + search: "is:unread repo:owner/repo", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: true, + wantIncludeBookmarked: false, + }, + { + name: "filter at end of string", + search: "some text is:read", + wantReadState: data.NotificationStateRead, + wantIsDone: false, + wantExplicitUnread: false, + wantIncludeBookmarked: false, + }, + { + name: "is:done with is:all", + search: "is:done is:all", + wantReadState: data.NotificationStateAll, + wantIsDone: true, + wantExplicitUnread: false, + wantIncludeBookmarked: false, + }, + { + name: "duplicate is:unread", + search: "is:unread is:unread", + wantReadState: data.NotificationStateUnread, + wantIsDone: false, + wantExplicitUnread: true, + wantIncludeBookmarked: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filters := parseNotificationFilters(tt.search) + + if filters.ReadState != tt.wantReadState { + t.Errorf("ReadState = %v, want %v", filters.ReadState, tt.wantReadState) + } + if filters.IsDone != tt.wantIsDone { + t.Errorf("IsDone = %v, want %v", filters.IsDone, tt.wantIsDone) + } + if filters.ExplicitUnread != tt.wantExplicitUnread { + t.Errorf("ExplicitUnread = %v, want %v", filters.ExplicitUnread, tt.wantExplicitUnread) + } + if filters.IncludeBookmarked != tt.wantIncludeBookmarked { + t.Errorf("IncludeBookmarked = %v, want %v", filters.IncludeBookmarked, tt.wantIncludeBookmarked) + } + }) + } +} + +func TestParseReasonFiltersEdgeCases(t *testing.T) { + tests := []struct { + name string + search string + expected []string + }{ + { + name: "reason with mixed case passes through as-is", + search: "reason:AUTHOR", + expected: []string{"AUTHOR"}, // Case is preserved, not normalized to lowercase + }, + { + name: "reason:participating with other filters", + search: "is:unread reason:participating repo:owner/repo", + expected: []string{ + data.ReasonAuthor, + data.ReasonComment, + data.ReasonMention, + data.ReasonReviewRequested, + data.ReasonAssign, + data.ReasonStateChange, + }, + }, + { + name: "duplicate reason filters", + search: "reason:author reason:author", + expected: []string{data.ReasonAuthor, data.ReasonAuthor}, + }, + { + name: "empty reason value", + search: "reason:", + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseReasonFilters(tt.search) + + if len(result) != len(tt.expected) { + t.Errorf("parseReasonFilters(%q) returned %d items, want %d", tt.search, len(result), len(tt.expected)) + t.Errorf("got: %v", result) + t.Errorf("want: %v", tt.expected) + return + } + + for i, want := range tt.expected { + if result[i] != want { + t.Errorf("parseReasonFilters(%q)[%d] = %q, want %q", tt.search, i, result[i], want) + } + } + }) + } +} diff --git a/internal/tui/components/notificationssection/notificationssection.go b/internal/tui/components/notificationssection/notificationssection.go new file mode 100644 index 000000000..14eb8f30e --- /dev/null +++ b/internal/tui/components/notificationssection/notificationssection.go @@ -0,0 +1,1049 @@ +package notificationssection + +import ( + "fmt" + "regexp" + "slices" + "sync" + "time" + + "github.com/charmbracelet/bubbles/key" + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/log" + + "github.com/dlvhdr/gh-dash/v4/internal/config" + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/notificationrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/section" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/table" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/keys" + "github.com/dlvhdr/gh-dash/v4/internal/utils" +) + +const SectionType = "notification" + +// repoFilterRegex matches "repo:owner/name" patterns in search strings +var repoFilterRegex = regexp.MustCompile(`repo:([^\s]+)`) + +// stateFilterRegex matches "is:unread", "is:read", "is:done", "is:all" patterns +var stateFilterRegex = regexp.MustCompile(`is:(unread|read|done|all)`) + +// reasonFilterRegex matches "reason:value" patterns in search strings +var reasonFilterRegex = regexp.MustCompile(`reason:([^\s]+)`) + +// parseRepoFilters extracts repo:owner/name patterns from a search string +func parseRepoFilters(search string) []string { + matches := repoFilterRegex.FindAllStringSubmatch(search, -1) + repos := make([]string, 0, len(matches)) + for _, match := range matches { + if len(match) > 1 { + repos = append(repos, match[1]) + } + } + return repos +} + +// NotificationFilters holds parsed notification filters +type NotificationFilters struct { + RepoFilters []string + ReasonFilters []string // Notification reasons to filter by (e.g., "author", "mention") + ReadState data.NotificationReadState + IsDone bool // If true, user asked for is:done which is not retrievable + ExplicitUnread bool // If true, user explicitly typed "is:unread" (excludes bookmarked+read) + IncludeBookmarked bool // If true, include bookmarked items even if read (default view) +} + +// parseReasonFilters extracts reason:value patterns from a search string +// Handles "reason:participating" as a meta-filter and normalizes hyphenated names +func parseReasonFilters(search string) []string { + matches := reasonFilterRegex.FindAllStringSubmatch(search, -1) + reasons := make([]string, 0, len(matches)) + for _, match := range matches { + if len(match) > 1 { + reason := match[1] + // Expand "participating" meta-filter to multiple reasons + if reason == "participating" { + reasons = append(reasons, + data.ReasonAuthor, + data.ReasonComment, + data.ReasonMention, + data.ReasonReviewRequested, + data.ReasonAssign, + data.ReasonStateChange, + ) + } else { + // Normalize hyphenated names to match GitHub API values + switch reason { + case "review-requested": + reasons = append(reasons, data.ReasonReviewRequested) + case "team-mention": + reasons = append(reasons, data.ReasonTeamMention) + case "ci-activity": + reasons = append(reasons, data.ReasonCIActivity) + case "security-alert": + reasons = append(reasons, data.ReasonSecurityAlert) + case "state-change": + reasons = append(reasons, data.ReasonStateChange) + default: + reasons = append(reasons, reason) + } + } + } + } + return reasons +} + +// parseNotificationFilters extracts all notification filters from search string +func parseNotificationFilters(search string) NotificationFilters { + filters := NotificationFilters{ + RepoFilters: parseRepoFilters(search), + ReasonFilters: parseReasonFilters(search), + ReadState: data.NotificationStateUnread, // Default to unread + IsDone: false, + ExplicitUnread: false, + IncludeBookmarked: true, // Default view includes bookmarked items + } + + matches := stateFilterRegex.FindAllStringSubmatch(search, -1) + hasUnread := false + hasRead := false + hasDone := false + hasAll := false + + for _, match := range matches { + if len(match) > 1 { + switch match[1] { + case "unread": + hasUnread = true + case "read": + hasRead = true + case "done": + hasDone = true + case "all": + hasAll = true + } + } + } + + if hasDone { + filters.IsDone = true + } + + if hasAll || (hasUnread && hasRead) { + filters.ReadState = data.NotificationStateAll + filters.IncludeBookmarked = false // Explicit filter, don't auto-include bookmarks + } else if hasRead { + filters.ReadState = data.NotificationStateRead + filters.IncludeBookmarked = false // Explicit filter, don't auto-include bookmarks + } else if hasUnread { + // User explicitly typed "is:unread" - don't include bookmarked+read items + filters.ReadState = data.NotificationStateUnread + filters.ExplicitUnread = true + filters.IncludeBookmarked = false + } + // Default case: ReadState = Unread, IncludeBookmarked = true + + return filters +} + +type SortOrder int + +const ( + SortByUpdated SortOrder = iota + SortByRepo +) + +type Model struct { + section.BaseModel + Notifications []notificationrow.Data + SortOrder SortOrder + lastSidebarOpen bool + sessionMarkedRead map[string]bool // IDs of notifications marked as read this session (kept visible until manual refresh) + sessionMarkedDone map[string]bool // IDs of notifications marked as done this session (excluded until manual refresh) +} + +func NewModel( + id int, + ctx *context.ProgramContext, + cfg config.NotificationsSectionConfig, + lastUpdated time.Time, +) Model { + sectionCfg := cfg.ToSectionConfig() + + m := Model{} + m.BaseModel = section.NewModel( + ctx, + section.NewSectionOptions{ + Id: id, + Config: sectionCfg, + Type: SectionType, + Columns: GetSectionColumns(ctx), + Singular: m.GetItemSingularForm(), + Plural: m.GetItemPluralForm(), + LastUpdated: lastUpdated, + CreatedAt: lastUpdated, + }, + ) + // Set 3-line content height for notification rows + m.Table.SetContentHeight(3) + // Respect smartFilteringAtLaunch - scope to current repo by default if enabled + m.IsFilteredByCurrentRemote = ctx.Config.SmartFilteringAtLaunch + m.SearchValue = m.GetSearchValue() + m.SearchBar.SetValue(m.SearchValue) + m.Notifications = []notificationrow.Data{} + m.sessionMarkedRead = make(map[string]bool) + m.sessionMarkedDone = make(map[string]bool) + + return m +} + +func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { + var cmd tea.Cmd + + switch msg := msg.(type) { + case tea.KeyMsg: + if m.IsSearchFocused() { + switch msg.Type { + case tea.KeyCtrlC, tea.KeyEsc: + m.SearchBar.SetValue(m.SearchValue) + blinkCmd := m.SetIsSearching(false) + return m, blinkCmd + + case tea.KeyEnter: + m.SearchValue = m.SearchBar.Value() + m.SetIsSearching(false) + m.ResetRows() + return m, tea.Batch(m.FetchNextPageSectionRows()...) + } + + break + } + + if m.IsPromptConfirmationFocused() { + switch msg.Type { + case tea.KeyCtrlC, tea.KeyEsc: + m.PromptConfirmationBox.Reset() + cmd = m.SetIsPromptConfirmationShown(false) + return m, cmd + + case tea.KeyEnter: + input := m.PromptConfirmationBox.Value() + action := m.GetPromptConfirmationAction() + if input == "Y" || input == "y" { + switch action { + case "done": + cmd = m.markAsDone() + case "done_all": + cmd = m.markAllAsDone() + } + } + + m.PromptConfirmationBox.Reset() + blinkCmd := m.SetIsPromptConfirmationShown(false) + + return m, tea.Batch(cmd, blinkCmd) + } + break + } + + switch { + case key.Matches(msg, keys.NotificationKeys.MarkAsDone): + if m.GetCurrRow() != nil { + cmd = m.markAsDone() + } + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.MarkAsRead): + if m.GetCurrRow() != nil { + cmd = m.markAsRead() + m.NextRow() + } + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.MarkAllAsRead): + cmd = m.markAllAsRead() + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.Unsubscribe): + if m.GetCurrRow() != nil { + cmd = m.unsubscribe() + } + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.Open): + if m.GetCurrRow() != nil { + cmd = m.openInBrowser() + } + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.ToggleBookmark): + if notification := m.GetCurrNotification(); notification != nil { + data.GetBookmarkStore().ToggleBookmark(notification.GetId()) + // Rebuild rows to update bookmark indicator + m.Table.SetRows(m.BuildRows()) + } + return m, nil + + case key.Matches(msg, keys.NotificationKeys.SortByRepo): + m.toggleSortOrder() + m.Table.SetRows(m.BuildRows()) + return m, nil + + case key.Matches(msg, keys.NotificationKeys.ToggleSmartFiltering): + if !m.HasRepoNameInConfiguredFilter() { + m.IsFilteredByCurrentRemote = !m.IsFilteredByCurrentRemote + } + searchValue := m.GetSearchValue() + if m.SearchValue != searchValue { + m.SearchValue = searchValue + m.SearchBar.SetValue(searchValue) + m.SetIsSearching(false) + m.ResetRows() + return m, tea.Batch(m.FetchNextPageSectionRows()...) + } + } + + case UpdateNotificationMsg: + if msg.IsRemoved { + for i, n := range m.Notifications { + if n.GetId() == msg.Id { + m.Notifications = append(m.Notifications[:i], m.Notifications[i+1:]...) + break + } + } + // Track as done so it doesn't reappear on refresh (GitHub API still returns it with all=true) + m.sessionMarkedDone[msg.Id] = true + // Also remove from sessionMarkedRead + delete(m.sessionMarkedRead, msg.Id) + m.TotalCount = len(m.Notifications) + m.SetIsLoading(false) + m.Table.SetRows(m.BuildRows()) + m.UpdateTotalItemsCount(m.TotalCount) + } + + case UpdateNotificationReadStateMsg: + // Update the notification's read state + for i := range m.Notifications { + if m.Notifications[i].GetId() == msg.Id { + m.Notifications[i].Notification.Unread = msg.Unread + // Track notifications marked as read this session so they remain visible + if !msg.Unread { + m.sessionMarkedRead[msg.Id] = true + } + m.Table.SetRows(m.BuildRows()) + break + } + } + + case UpdateNotificationCommentsMsg: + // Update the notification with fetched data + log.Debug("UpdateNotificationCommentsMsg received", "id", msg.Id, "count", msg.NewCommentsCount, "state", msg.SubjectState, "actor", msg.Actor) + for i := range m.Notifications { + if m.Notifications[i].GetId() == msg.Id { + m.Notifications[i].NewCommentsCount = msg.NewCommentsCount + m.Notifications[i].SubjectState = msg.SubjectState + m.Notifications[i].IsDraft = msg.IsDraft + m.Notifications[i].Actor = msg.Actor + // Generate activity description based on reason, type, and actor + m.Notifications[i].ActivityDescription = notificationrow.GenerateActivityDescription( + m.Notifications[i].GetReason(), + m.Notifications[i].GetSubjectType(), + msg.Actor, + ) + m.Table.SetRows(m.BuildRows()) + log.Debug("Updated notification", "id", msg.Id, "count", msg.NewCommentsCount, "state", msg.SubjectState, "actor", msg.Actor) + break + } + } + + case UpdateNotificationUrlMsg: + // Update the notification with async-resolved URL (e.g., for CheckSuite) + log.Debug("UpdateNotificationUrlMsg received", "id", msg.Id, "url", msg.ResolvedUrl) + for i := range m.Notifications { + if m.Notifications[i].GetId() == msg.Id { + m.Notifications[i].ResolvedUrl = msg.ResolvedUrl + m.Table.SetRows(m.BuildRows()) + log.Debug("Updated notification URL", "id", msg.Id, "url", msg.ResolvedUrl) + break + } + } + + case SectionNotificationsFetchedMsg: + if m.LastFetchTaskId == msg.TaskId { + if m.PageInfo != nil { + // Append to existing notifications (pagination) + m.Notifications = append(m.Notifications, msg.Notifications...) + } else { + // First page, replace + m.Notifications = msg.Notifications + } + m.TotalCount = len(m.Notifications) + m.PageInfo = &msg.PageInfo + m.SetIsLoading(false) + m.Table.SetRows(m.BuildRows()) + m.UpdateLastUpdated(time.Now()) + m.UpdateTotalItemsCount(m.TotalCount) + + // Start background fetches for comment counts (only for new notifications) + fetchCmds := m.fetchCommentCountsForNotifications(msg.Notifications) + cmd = tea.Batch(fetchCmds...) + } + + case ClearAllNotificationsMsg: + // Clear all notifications after marking all as done + m.Notifications = []notificationrow.Data{} + m.TotalCount = 0 + m.SetIsLoading(false) + m.Table.SetRows(m.BuildRows()) + m.UpdateTotalItemsCount(0) + + case MarkAllAsReadMsg: + // Mark all notifications as read (update their state) + for i := range m.Notifications { + m.Notifications[i].Notification.Unread = false + // Track all as marked read this session so they remain visible + m.sessionMarkedRead[m.Notifications[i].GetId()] = true + } + m.Table.SetRows(m.BuildRows()) + } + + search, searchCmd := m.SearchBar.Update(msg) + m.SearchBar = search + + prompt, promptCmd := m.PromptConfirmationBox.Update(msg) + m.PromptConfirmationBox = prompt + + tbl, tableCmd := m.Table.Update(msg) + m.Table = tbl + + return m, tea.Batch(cmd, searchCmd, promptCmd, tableCmd) +} + +func GetSectionColumns(ctx *context.ProgramContext) []table.Column { + return []table.Column{ + { + Title: "Type", + Width: utils.IntPtr(6), // Type icon + Align: func() *lipgloss.Position { p := lipgloss.Center; return &p }(), + }, + { + Title: "Title", + Grow: utils.BoolPtr(true), // 3-line title block (includes bookmark icon) + }, + { + Title: "Activity", + Width: utils.IntPtr(10), // Comments count with icon + Align: func() *lipgloss.Position { p := lipgloss.Right; return &p }(), + }, + { + Title: "󱦻 ", // Trailing padding to center when right-aligned + Width: utils.IntPtr(12), // Updated at (e.g., "12mo ago") + Align: func() *lipgloss.Position { p := lipgloss.Right; return &p }(), + }, + } +} + +func (m *Model) toggleSortOrder() { + if m.SortOrder == SortByUpdated { + m.SortOrder = SortByRepo + } else { + m.SortOrder = SortByUpdated + } + m.sortNotifications() +} + +func (m *Model) sortNotifications() { + switch m.SortOrder { + case SortByRepo: + slices.SortFunc(m.Notifications, func(a, b notificationrow.Data) int { + repoA := a.Notification.Repository.FullName + repoB := b.Notification.Repository.FullName + if repoA < repoB { + return -1 + } + if repoA > repoB { + return 1 + } + // Secondary sort by updated time (most recent first) + if a.Notification.UpdatedAt.After(b.Notification.UpdatedAt) { + return -1 + } + if a.Notification.UpdatedAt.Before(b.Notification.UpdatedAt) { + return 1 + } + return 0 + }) + case SortByUpdated: + slices.SortFunc(m.Notifications, func(a, b notificationrow.Data) int { + if a.Notification.UpdatedAt.After(b.Notification.UpdatedAt) { + return -1 + } + if a.Notification.UpdatedAt.Before(b.Notification.UpdatedAt) { + return 1 + } + return 0 + }) + } +} + +func (m Model) BuildRows() []table.Row { + var rows []table.Row + for i := range m.Notifications { + notification := &m.Notifications[i] + notificationModel := notificationrow.Notification{Ctx: m.Ctx, Data: notification} + rows = append(rows, notificationModel.ToTableRow()) + } + + if rows == nil { + rows = []table.Row{} + } + + return rows +} + +func (m *Model) NumRows() int { + return len(m.Notifications) +} + +func (m *Model) GetCurrRow() data.RowData { + idx := m.Table.GetCurrItem() + if idx < 0 || idx >= len(m.Notifications) { + return nil + } + return &m.Notifications[idx] +} + +func (m *Model) GetCurrNotification() *notificationrow.Data { + idx := m.Table.GetCurrItem() + if idx < 0 || idx >= len(m.Notifications) { + return nil + } + return &m.Notifications[idx] +} + +func (m *Model) FetchNextPageSectionRows() []tea.Cmd { + if m == nil { + return nil + } + + // Check if there's a next page (skip if we already know there isn't) + if m.PageInfo != nil && !m.PageInfo.HasNextPage { + return nil + } + + var cmds []tea.Cmd + + // Parse filters from search value (includes repo filter if smartFilteringAtLaunch is enabled) + filters := parseNotificationFilters(m.GetSearchValue()) + + // Handle is:done filter - these notifications cannot be retrieved + if filters.IsDone { + m.Notifications = []notificationrow.Data{} + m.TotalCount = 0 + m.SetIsLoading(false) + m.Table.SetRows(m.BuildRows()) + m.UpdateTotalItemsCount(0) + // Return a message that will be shown to the user + return []tea.Cmd{func() tea.Msg { + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: m.Type, + TaskId: "done_filter", + Err: fmt.Errorf("done notifications cannot be retrieved"), + } + }} + } + + taskId := fmt.Sprintf("fetching_notifications_%d_%s", m.Id, time.Now().String()) + m.LastFetchTaskId = taskId + task := context.Task{ + Id: taskId, + StartText: "Fetching notifications", + FinishedText: "Notifications have been fetched", + State: context.TaskStart, + Error: nil, + } + startCmd := m.Ctx.StartTask(task) + cmds = append(cmds, startCmd) + + // Capture session state for the closure + sessionMarkedRead := m.sessionMarkedRead + hasSessionMarkedRead := len(sessionMarkedRead) > 0 + sessionMarkedDone := m.sessionMarkedDone + + // Capture current page info for pagination + pageInfo := m.PageInfo + + // Capture config limit for the closure + limit := m.Ctx.Config.Defaults.NotificationsLimit + + // Build reason filter map for O(1) lookup + reasonFilterMap := make(map[string]bool, len(filters.ReasonFilters)) + for _, reason := range filters.ReasonFilters { + reasonFilterMap[reason] = true + } + + fetchCmd := func() tea.Msg { + // Check if we need to include bookmarked items + // Build a map for O(1) lookups in the filter loop + bookmarkStore := data.GetBookmarkStore() + bookmarkedIds := bookmarkStore.GetBookmarkedIds() + hasBookmarks := len(bookmarkedIds) > 0 + bookmarkedIdMap := make(map[string]bool, len(bookmarkedIds)) + for _, id := range bookmarkedIds { + bookmarkedIdMap[id] = true + } + + // Use the filter's read state directly - don't switch to "all" just for bookmarks/session items + // Bookmarked and session-marked-read items will be fetched separately by thread ID + readState := filters.ReadState + + // Initialize done store for filtering + doneStore := data.GetDoneStore() + + // Track accumulated notifications across multiple pages. + // We may need to fetch additional pages if many notifications are filtered out + // (e.g., marked as done locally). The loop continues until we have enough + // notifications to display or run out of pages from the API. + notifications := make([]notificationrow.Data, 0, limit) + currentPageInfo := pageInfo + var lastPageInfo data.PageInfo + isFirstPage := pageInfo == nil + for { + res, err := data.FetchNotifications(limit, filters.RepoFilters, readState, currentPageInfo) + if err != nil { + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: m.Type, + TaskId: taskId, + Err: err, + } + } + lastPageInfo = res.PageInfo + + // Build a set of IDs we fetched from the API + fetchedIds := make(map[string]bool, len(res.Notifications)) + for _, n := range res.Notifications { + fetchedIds[n.Id] = true + } + + // On first page, fetch any bookmarked/session-marked-read notifications that are missing + // (they may have aged out of the default notifications list or been marked as read) + if isFirstPage { + isFirstPage = false + + // Collect all missing IDs that need to be fetched + missingIds := make([]string, 0) + if filters.IncludeBookmarked && hasBookmarks { + for _, bookmarkId := range bookmarkedIds { + if !fetchedIds[bookmarkId] { + missingIds = append(missingIds, bookmarkId) + fetchedIds[bookmarkId] = true // Mark as fetched to avoid duplicates + } + } + } + if hasSessionMarkedRead { + for id := range sessionMarkedRead { + if !fetchedIds[id] { + missingIds = append(missingIds, id) + fetchedIds[id] = true + } + } + } + + // Fetch all missing notifications in parallel + if len(missingIds) > 0 { + // Build repo filter map for O(1) lookup + repoFilterMap := make(map[string]bool, len(filters.RepoFilters)) + for _, repo := range filters.RepoFilters { + repoFilterMap[repo] = true + } + + type fetchResult struct { + notification *data.NotificationData + err error + } + results := make(chan fetchResult, len(missingIds)) + + var wg sync.WaitGroup + for _, id := range missingIds { + wg.Add(1) + go func(threadId string) { + defer wg.Done() + notification, err := data.FetchNotificationByThreadId(threadId) + results <- fetchResult{notification: notification, err: err} + }(id) + } + + // Close results channel when all goroutines complete + go func() { + wg.Wait() + close(results) + }() + + // Collect results + for result := range results { + if result.err != nil { + log.Debug("Failed to fetch missing notification", "err", result.err) + continue + } + if result.notification == nil { + continue + } + // Apply repo filter if set + if len(repoFilterMap) > 0 && !repoFilterMap[result.notification.Repository.FullName] { + continue + } + res.Notifications = append(res.Notifications, *result.notification) + } + } + } + + // Filter notifications based on bookmark settings and session state + for _, n := range res.Notifications { + // Skip notifications marked as done (GitHub API still returns them with all=true) + // Check both persistent store and session state + if doneStore.IsDone(n.Id) || sessionMarkedDone[n.Id] { + continue + } + + include := false + + // Always include notifications marked as read this session (until manual refresh) + if sessionMarkedRead[n.Id] { + include = true + } else if filters.IncludeBookmarked && hasBookmarks { + // Default view: include if unread OR bookmarked (O(1) map lookup) + include = n.Unread || bookmarkedIdMap[n.Id] + } else { + // Explicit filter: follow the ReadState filter + switch filters.ReadState { + case data.NotificationStateUnread: + include = n.Unread + case data.NotificationStateRead: + include = !n.Unread + case data.NotificationStateAll: + include = true + } + } + + // Apply reason filter if specified (O(1) map lookup) + if include && len(reasonFilterMap) > 0 { + include = reasonFilterMap[n.Reason] + } + + if include { + notifications = append(notifications, notificationrow.Data{ + Notification: n, + // Generate initial activity description (will be updated with actor later) + ActivityDescription: notificationrow.GenerateActivityDescription(n.Reason, n.Subject.Type, ""), + }) + } + } + + // Check if we have enough notifications or if we've run out of pages + if len(notifications) >= limit || !lastPageInfo.HasNextPage { + break + } + + // Need more notifications - fetch the next page + currentPageInfo = &lastPageInfo + log.Debug("Fetching additional page due to done filtering", + "currentCount", len(notifications), + "targetLimit", limit, + "nextPage", lastPageInfo.EndCursor) + } + + return constants.TaskFinishedMsg{ + SectionId: m.Id, + SectionType: m.Type, + TaskId: taskId, + Msg: SectionNotificationsFetchedMsg{ + Notifications: notifications, + TotalCount: len(notifications), + TaskId: taskId, + PageInfo: lastPageInfo, + }, + } + } + cmds = append(cmds, fetchCmd) + + return cmds +} + +func (m *Model) UpdateLastUpdated(t time.Time) { + m.Table.UpdateLastUpdated(t) +} + +func (m *Model) ResetRows() { + m.Notifications = nil + // Clear session state on manual refresh - user explicitly wants fresh data + m.sessionMarkedRead = make(map[string]bool) + m.sessionMarkedDone = make(map[string]bool) + m.BaseModel.ResetRows() +} + +// FetchAllSections creates and fetches all notification sections based on config. +// Returns sections and a batch command to fetch all data. +func FetchAllSections( + ctx *context.ProgramContext, + existing []section.Section, +) (sections []section.Section, fetchAllCmd tea.Cmd) { + sectionConfigs := ctx.Config.NotificationsSections + fetchCmds := make([]tea.Cmd, 0, len(sectionConfigs)) + sections = make([]section.Section, 0, len(sectionConfigs)) + + for i, sectionConfig := range sectionConfigs { + sectionModel := NewModel( + i+1, // ID 0 is reserved for search section + ctx, + sectionConfig, + time.Now(), + ) + + // Preserve existing data and filter state if refreshing + if len(existing) > i+1 && existing[i+1] != nil { + if oldSection, ok := existing[i+1].(*Model); ok { + sectionModel.Notifications = oldSection.Notifications + sectionModel.LastFetchTaskId = oldSection.LastFetchTaskId + sectionModel.sessionMarkedRead = oldSection.sessionMarkedRead + sectionModel.sessionMarkedDone = oldSection.sessionMarkedDone + // Preserve user's filter state - don't reset on refresh + sectionModel.IsFilteredByCurrentRemote = oldSection.IsFilteredByCurrentRemote + sectionModel.SearchValue = oldSection.SearchValue + sectionModel.SearchBar.SetValue(oldSection.SearchValue) + } + } + + sections = append(sections, §ionModel) + fetchCmds = append(fetchCmds, sectionModel.FetchNextPageSectionRows()...) + } + + return sections, tea.Batch(fetchCmds...) +} + +// SectionNotificationsFetchedMsg contains the result of fetching notifications from the GitHub API. +// This message is sent when the initial fetch or a refresh completes. +type SectionNotificationsFetchedMsg struct { + Notifications []notificationrow.Data + TotalCount int + TaskId string + PageInfo data.PageInfo +} + +// UpdateNotificationMsg signals that a notification's state has changed. +// If IsRemoved is true, the notification should be removed from the list (marked as done). +type UpdateNotificationMsg struct { + Id string + IsRemoved bool +} + +// UpdateNotificationCommentsMsg carries additional notification metadata fetched asynchronously. +// This includes comment counts, PR/Issue state, draft status, and the actor who triggered the notification. +type UpdateNotificationCommentsMsg struct { + Id string + NewCommentsCount int + SubjectState string // OPEN, CLOSED, MERGED + IsDraft bool + Actor string // Username who triggered the notification +} + +// UpdateNotificationUrlMsg carries a resolved URL for notifications where the URL +// cannot be determined synchronously (e.g., CheckSuite notifications). +type UpdateNotificationUrlMsg struct { + Id string + ResolvedUrl string +} + +func (m Model) GetItemSingularForm() string { + return "Notification" +} + +func (m Model) GetItemPluralForm() string { + return "Notifications" +} + +func (m Model) GetTotalCount() int { + return m.TotalCount +} + +func (m *Model) GetIsLoading() bool { + return m.IsLoading +} + +func (m *Model) SetIsLoading(val bool) { + m.IsLoading = val + m.Table.SetIsLoading(val) +} + +func (m Model) GetPagerContent() string { + pagerContent := "" + if m.TotalCount > 0 { + pagerContent = fmt.Sprintf( + "%v %v • %v %v/%v", + constants.WaitingIcon, + m.LastUpdated().Format("01/02 15:04:05"), + m.SingularForm, + m.Table.GetCurrItem()+1, + m.TotalCount, + ) + } + pager := m.Ctx.Styles.ListViewPort.PagerStyle.Render(pagerContent) + return pager +} + +func (m *Model) UpdateProgramContext(ctx *context.ProgramContext) { + if ctx == nil { + return + } + + // Rebuild columns if sidebar state changed + if ctx.SidebarOpen != m.lastSidebarOpen { + m.lastSidebarOpen = ctx.SidebarOpen + m.Table.Columns = GetSectionColumns(ctx) + } + + m.BaseModel.UpdateProgramContext(ctx) +} + +// fetchCommentCountsForNotifications returns commands to fetch comment counts for the given notifications +func (m *Model) fetchCommentCountsForNotifications(notifications []notificationrow.Data) []tea.Cmd { + var cmds []tea.Cmd + + log.Debug("fetchCommentCountsForNotifications called", "numNotifications", len(notifications)) + + for _, notif := range notifications { + // Copy values for closure capture + notifId := notif.GetId() + subjectType := notif.GetSubjectType() + subjectUrl := notif.GetUrl() + lastReadAt := notif.Notification.LastReadAt + apiUrl := notif.Notification.Subject.Url + + log.Debug("Processing notification", "id", notifId, "type", subjectType, "webUrl", subjectUrl, "apiUrl", apiUrl) + + latestCommentUrl := notif.Notification.Subject.LatestCommentUrl + + // Only fetch for PR and Issue types + switch subjectType { + case "PullRequest": + // Capture variables for closure + id, url, readAt, commentUrl := notifId, subjectUrl, lastReadAt, latestCommentUrl + cmds = append(cmds, func() tea.Msg { + log.Debug("Fetching PR for comment count", "url", url) + pr, err := data.FetchPullRequest(url) + if err != nil { + log.Error("Failed to fetch PR for comment count", "url", url, "err", err) + return nil + } + count := countNewPRComments(pr, readAt) + actor, _ := data.FetchCommentAuthor(commentUrl) + if actor == "" { + actor = pr.Author.Login + } + log.Debug("Got PR comment count", "id", id, "count", count, "state", pr.State, "actor", actor) + return UpdateNotificationCommentsMsg{ + Id: id, + NewCommentsCount: count, + SubjectState: pr.State, + IsDraft: pr.IsDraft, + Actor: actor, + } + }) + case "Issue": + // Capture variables for closure + id, url, readAt, commentUrl := notifId, subjectUrl, lastReadAt, latestCommentUrl + cmds = append(cmds, func() tea.Msg { + log.Debug("Fetching Issue for comment count", "url", url) + issue, err := data.FetchIssue(url) + if err != nil { + log.Error("Failed to fetch Issue for comment count", "url", url, "err", err) + return nil + } + count := countNewIssueComments(issue, readAt) + actor, _ := data.FetchCommentAuthor(commentUrl) + if actor == "" { + actor = issue.Author.Login + } + log.Debug("Got Issue comment count", "id", id, "count", count, "state", issue.State, "actor", actor) + return UpdateNotificationCommentsMsg{ + Id: id, + NewCommentsCount: count, + SubjectState: issue.State, + Actor: actor, + } + }) + case "CheckSuite": + // CheckSuite notifications have subject.url=null in GitHub's API. + // We fetch recent workflow runs and find the best match by timestamp. + id := notifId + repo := notif.Notification.Repository.FullName + updatedAt := notif.Notification.UpdatedAt + title := notif.Notification.Subject.Title + cmds = append(cmds, func() tea.Msg { + log.Debug("Fetching workflow run for CheckSuite", "id", id, "repo", repo) + url, err := data.FetchRecentWorkflowRun(repo, updatedAt, title) + if err != nil { + log.Error("Failed to fetch workflow run", "id", id, "err", err) + return nil + } + if url == "" { + log.Debug("No matching workflow run found", "id", id) + return nil + } + log.Debug("Found workflow run URL", "id", id, "url", url) + return UpdateNotificationUrlMsg{ + Id: id, + ResolvedUrl: url, + } + }) + } + } + + log.Debug("fetchCommentCountsForNotifications returning", "numCmds", len(cmds)) + return cmds +} + +// countNewPRComments counts comments in a PR that are newer than lastReadAt +// If lastReadAt is nil (never read), counts all comments +func countNewPRComments(pr data.EnrichedPullRequestData, lastReadAt *time.Time) int { + count := 0 + + for _, comment := range pr.Comments.Nodes { + if lastReadAt == nil || comment.UpdatedAt.After(*lastReadAt) { + count++ + } + } + + for _, thread := range pr.ReviewThreads.Nodes { + for _, comment := range thread.Comments.Nodes { + if lastReadAt == nil || comment.UpdatedAt.After(*lastReadAt) { + count++ + } + } + } + + for _, review := range pr.Reviews.Nodes { + if lastReadAt == nil || review.UpdatedAt.After(*lastReadAt) { + count++ + } + } + + return count +} + +// countNewIssueComments counts comments in an Issue that are newer than lastReadAt +// If lastReadAt is nil (never read), counts all comments +func countNewIssueComments(issue data.IssueData, lastReadAt *time.Time) int { + count := 0 + for _, comment := range issue.Comments.Nodes { + if lastReadAt == nil || comment.UpdatedAt.After(*lastReadAt) { + count++ + } + } + + return count +} diff --git a/internal/tui/components/notificationview/notificationview.go b/internal/tui/components/notificationview/notificationview.go new file mode 100644 index 000000000..ac5be87fc --- /dev/null +++ b/internal/tui/components/notificationview/notificationview.go @@ -0,0 +1,234 @@ +package notificationview + +import ( + "fmt" + "strings" + + "github.com/charmbracelet/lipgloss" + + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/common" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/notificationrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" +) + +type Model struct { + ctx *context.ProgramContext + row *notificationrow.Data + width int + + // Cached notification subject data for sidebar display + subjectPR *prrow.Data + subjectIssue *data.IssueData + subjectId string // ID of the notification whose subject is cached +} + +func NewModel(ctx *context.ProgramContext) Model { + return Model{ + ctx: ctx, + } +} + +func (m *Model) SetRow(row *notificationrow.Data) { + m.row = row +} + +func (m *Model) SetWidth(width int) { + m.width = width +} + +func (m *Model) SetSubjectPR(pr *prrow.Data, notificationId string) { + m.subjectPR = pr + m.subjectIssue = nil + m.subjectId = notificationId +} + +func (m *Model) SetSubjectIssue(issue *data.IssueData, notificationId string) { + m.subjectIssue = issue + m.subjectPR = nil + m.subjectId = notificationId +} + +func (m *Model) GetSubjectPR() *prrow.Data { + return m.subjectPR +} + +func (m *Model) GetSubjectIssue() *data.IssueData { + return m.subjectIssue +} + +func (m *Model) GetSubjectId() string { + return m.subjectId +} + +func (m *Model) UpdateProgramContext(ctx *context.ProgramContext) { + m.ctx = ctx +} + +func (m Model) View() string { + if m.row == nil { + return "" + } + + s := strings.Builder{} + notification := m.row.Notification + + // Title - using common preview styling + titleBlock := common.RenderPreviewTitle(m.ctx.Theme, m.ctx.Styles.Common, m.width, notification.Subject.Title) + + labelStyle := lipgloss.NewStyle(). + Foreground(m.ctx.Theme.FaintText). + Width(16) + + valueStyle := lipgloss.NewStyle(). + Foreground(m.ctx.Theme.SecondaryText) + + faintValueStyle := lipgloss.NewStyle(). + Foreground(m.ctx.Theme.FaintText) + + sectionStyle := lipgloss.NewStyle(). + PaddingBottom(1) + + s.WriteString(titleBlock) + s.WriteString("\n\n") + + // Type with icon + typeIcon := getTypeIcon(notification.Subject.Type) + typeRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Type"), + valueStyle.Render(fmt.Sprintf("%s %s", typeIcon, notification.Subject.Type)), + ) + s.WriteString(sectionStyle.Render(typeRow)) + s.WriteString("\n") + + repoRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Repository"), + valueStyle.Render(notification.Repository.FullName), + ) + s.WriteString(sectionStyle.Render(repoRow)) + s.WriteString("\n") + + visibility := "Public" + if notification.Repository.Private { + visibility = "Private" + } + visibilityRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Visibility"), + valueStyle.Render(visibility), + ) + s.WriteString(sectionStyle.Render(visibilityRow)) + s.WriteString("\n") + + reasonRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Reason"), + valueStyle.Render(formatReason(notification.Reason)), + ) + s.WriteString(sectionStyle.Render(reasonRow)) + s.WriteString("\n") + + status := "Read" + if notification.Unread { + status = "Unread" + } + statusRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Status"), + valueStyle.Render(status), + ) + s.WriteString(sectionStyle.Render(statusRow)) + s.WriteString("\n") + + // Updated at + updatedRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Updated"), + valueStyle.Render(notification.UpdatedAt.Local().Format("Jan 2, 2006 3:04 PM")), + ) + s.WriteString(sectionStyle.Render(updatedRow)) + s.WriteString("\n") + + // Last read at + lastReadValue := "Never" + if notification.LastReadAt != nil { + lastReadValue = notification.LastReadAt.Local().Format("Jan 2, 2006 3:04 PM") + } + lastReadRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Last Read"), + valueStyle.Render(lastReadValue), + ) + s.WriteString(sectionStyle.Render(lastReadRow)) + s.WriteString("\n") + + hasComment := "No" + if notification.Subject.LatestCommentUrl != "" { + hasComment = "Yes" + } + commentRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Has Comment"), + valueStyle.Render(hasComment), + ) + s.WriteString(sectionStyle.Render(commentRow)) + s.WriteString("\n") + + idRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("Notification ID"), + faintValueStyle.Render(notification.Id), + ) + s.WriteString(sectionStyle.Render(idRow)) + s.WriteString("\n") + + if notification.Subject.Url != "" { + urlRow := lipgloss.JoinHorizontal(lipgloss.Top, + labelStyle.Render("API URL"), + faintValueStyle.Render(notification.Subject.Url), + ) + s.WriteString(sectionStyle.Render(urlRow)) + } + + return s.String() +} + +func getTypeIcon(subjectType string) string { + switch subjectType { + case "PullRequest": + return "" + case "Issue": + return "" + case "Discussion": + return "" + case "Release": + return "" + case "Commit": + return "" + case "CheckSuite": + return "" + default: + return "" + } +} + +func formatReason(reason string) string { + switch reason { + case "subscribed": + return "Subscribed" + case "review_requested": + return "Review requested" + case "author": + return "Author" + case "comment": + return "Comment" + case "mention": + return "Mentioned" + case "team_mention": + return "Team mentioned" + case "state_change": + return "State changed" + case "assign": + return "Assigned" + case "ci_activity": + return "CI activity" + case "approval_requested": + return "Approval requested" + default: + return reason + } +} diff --git a/internal/tui/components/prssection/diff.go b/internal/tui/components/prssection/diff.go index 9840d0e69..c09b04ddb 100644 --- a/internal/tui/components/prssection/diff.go +++ b/internal/tui/components/prssection/diff.go @@ -1,11 +1,9 @@ package prssection import ( - "fmt" - "os/exec" - tea "github.com/charmbracelet/bubbletea" - "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" + + "github.com/dlvhdr/gh-dash/v4/internal/tui/common" ) func (m Model) diff() tea.Cmd { @@ -14,20 +12,5 @@ func (m Model) diff() tea.Cmd { return nil } - c := exec.Command( - "gh", - "pr", - "diff", - fmt.Sprint(currRowData.GetNumber()), - "-R", - m.GetCurrRow().GetRepoNameWithOwner(), - ) - c.Env = m.Ctx.Config.GetFullScreenDiffPagerEnv() - - return tea.ExecProcess(c, func(err error) tea.Msg { - if err != nil { - return constants.ErrMsg{Err: err} - } - return nil - }) + return common.DiffPR(currRowData.GetNumber(), currRowData.GetRepoNameWithOwner(), m.Ctx.Config.GetFullScreenDiffPagerEnv()) } diff --git a/internal/tui/components/prview/action.go b/internal/tui/components/prview/action.go new file mode 100644 index 000000000..419a0d0e3 --- /dev/null +++ b/internal/tui/components/prview/action.go @@ -0,0 +1,69 @@ +package prview + +import ( + "github.com/charmbracelet/bubbles/key" + tea "github.com/charmbracelet/bubbletea" + + "github.com/dlvhdr/gh-dash/v4/internal/tui/keys" +) + +// PRActionType identifies the type of action requested by a key press in the PR view. +type PRActionType int + +const ( + PRActionNone PRActionType = iota + PRActionApprove + PRActionAssign + PRActionUnassign + PRActionComment + PRActionDiff + PRActionCheckout + PRActionClose + PRActionReady + PRActionReopen + PRActionMerge + PRActionUpdate + PRActionSummaryViewMore +) + +// PRAction represents an action to be performed on a PR. +type PRAction struct { + Type PRActionType +} + +// MsgToAction converts a tea.Msg to a PRAction if it matches a known key binding. +func MsgToAction(msg tea.Msg) *PRAction { + keyMsg, ok := msg.(tea.KeyMsg) + if !ok { + return nil + } + + switch { + case key.Matches(keyMsg, keys.PRKeys.Approve): + return &PRAction{Type: PRActionApprove} + case key.Matches(keyMsg, keys.PRKeys.Assign): + return &PRAction{Type: PRActionAssign} + case key.Matches(keyMsg, keys.PRKeys.Unassign): + return &PRAction{Type: PRActionUnassign} + case key.Matches(keyMsg, keys.PRKeys.Comment): + return &PRAction{Type: PRActionComment} + case key.Matches(keyMsg, keys.PRKeys.Diff): + return &PRAction{Type: PRActionDiff} + case key.Matches(keyMsg, keys.PRKeys.Checkout): + return &PRAction{Type: PRActionCheckout} + case key.Matches(keyMsg, keys.PRKeys.Close): + return &PRAction{Type: PRActionClose} + case key.Matches(keyMsg, keys.PRKeys.Ready): + return &PRAction{Type: PRActionReady} + case key.Matches(keyMsg, keys.PRKeys.Reopen): + return &PRAction{Type: PRActionReopen} + case key.Matches(keyMsg, keys.PRKeys.Merge): + return &PRAction{Type: PRActionMerge} + case key.Matches(keyMsg, keys.PRKeys.Update): + return &PRAction{Type: PRActionUpdate} + case key.Matches(keyMsg, keys.PRKeys.SummaryViewMore): + return &PRAction{Type: PRActionSummaryViewMore} + } + + return nil +} diff --git a/internal/tui/components/prview/action_test.go b/internal/tui/components/prview/action_test.go new file mode 100644 index 000000000..cd664995f --- /dev/null +++ b/internal/tui/components/prview/action_test.go @@ -0,0 +1,185 @@ +package prview + +import ( + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/require" + + "github.com/dlvhdr/gh-dash/v4/internal/config" + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/keys" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" +) + +func newTestModelForAction(t *testing.T) Model { + t.Helper() + cfg, err := config.ParseConfig(config.Location{ + ConfigFlag: "../../../config/testdata/test-config.yml", + }) + if err != nil { + t.Fatal(err) + } + thm := theme.ParseTheme(&cfg) + ctx := &context.ProgramContext{ + Config: &cfg, + Theme: thm, + Styles: context.InitStyles(thm), + } + + m := NewModel(ctx) + m.ctx = ctx + m.pr = &prrow.PullRequest{ + Ctx: ctx, + Data: &prrow.Data{ + Primary: &data.PullRequestData{}, + IsEnriched: true, + }, + } + return m +} + +func TestMsgToActionReturnsCorrectActions(t *testing.T) { + testCases := []struct { + name string + keyBinding string + expectedAction PRActionType + }{ + {"approve key", "v", PRActionApprove}, + {"assign key", "a", PRActionAssign}, + {"unassign key", "A", PRActionUnassign}, + {"comment key", "c", PRActionComment}, + {"diff key", "d", PRActionDiff}, + {"checkout key C", "C", PRActionCheckout}, + {"checkout key space", " ", PRActionCheckout}, + {"close key", "x", PRActionClose}, + {"ready key", "W", PRActionReady}, + {"reopen key", "X", PRActionReopen}, + {"merge key", "m", PRActionMerge}, + {"update key", "u", PRActionUpdate}, + {"summary view more key", "e", PRActionSummaryViewMore}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(tc.keyBinding)} + + action := MsgToAction(msg) + + require.NotNil(t, action, "expected action for key %q", tc.keyBinding) + require.Equal(t, tc.expectedAction, action.Type, + "expected action type %v for key %q, got %v", tc.expectedAction, tc.keyBinding, action.Type) + }) + } +} + +func TestMsgToActionReturnsNilForUnknownKeys(t *testing.T) { + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("z")} + + action := MsgToAction(msg) + + require.Nil(t, action, "expected nil action for unknown key") +} + +func TestIsTextInputBoxFocusedWhenCommenting(t *testing.T) { + m := newTestModelForAction(t) + m.isCommenting = true + + require.True(t, m.IsTextInputBoxFocused(), "expected text input box focused when in commenting mode") +} + +func TestIsTextInputBoxFocusedWhenApproving(t *testing.T) { + m := newTestModelForAction(t) + m.isApproving = true + + require.True(t, m.IsTextInputBoxFocused(), "expected text input box focused when in approving mode") +} + +func TestIsTextInputBoxFocusedWhenAssigning(t *testing.T) { + m := newTestModelForAction(t) + m.isAssigning = true + + require.True(t, m.IsTextInputBoxFocused(), "expected text input box focused when in assigning mode") +} + +func TestIsTextInputBoxFocusedWhenUnassigning(t *testing.T) { + m := newTestModelForAction(t) + m.isUnassigning = true + + require.True(t, m.IsTextInputBoxFocused(), "expected text input box focused when in unassigning mode") +} + +func TestUpdateHandlesSidebarTabNavigation(t *testing.T) { + t.Run("prev sidebar tab", func(t *testing.T) { + m := newTestModelForAction(t) + // Move to a non-first tab first + m.carousel.MoveRight() + initialTab := m.carousel.SelectedItem() + + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("[")} + m, _ = m.Update(msg) + + require.NotEqual(t, initialTab, m.carousel.SelectedItem(), + "carousel should have moved to previous tab") + }) + + t.Run("next sidebar tab", func(t *testing.T) { + m := newTestModelForAction(t) + initialTab := m.carousel.SelectedItem() + + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("]")} + m, _ = m.Update(msg) + + require.NotEqual(t, initialTab, m.carousel.SelectedItem(), + "carousel should have moved to next tab") + }) +} + +func TestPRActionTypes(t *testing.T) { + // Verify all action types are distinct + actionTypes := []PRActionType{ + PRActionNone, + PRActionApprove, + PRActionAssign, + PRActionUnassign, + PRActionComment, + PRActionDiff, + PRActionCheckout, + PRActionClose, + PRActionReady, + PRActionReopen, + PRActionMerge, + PRActionUpdate, + PRActionSummaryViewMore, + } + + seen := make(map[PRActionType]bool) + for _, at := range actionTypes { + require.False(t, seen[at], "duplicate action type value: %v", at) + seen[at] = true + } + + // Verify PRActionNone is zero value + require.Equal(t, PRActionType(0), PRActionNone, "PRActionNone should be zero value") +} + +func TestMsgToActionWithReboundKeys(t *testing.T) { + // Save original key bindings + originalApproveKeys := keys.PRKeys.Approve.Keys() + + // Rebind approve key to "V" (uppercase) + keys.PRKeys.Approve.SetKeys("V") + defer func() { + // Restore original bindings + keys.PRKeys.Approve.SetKeys(originalApproveKeys...) + }() + + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("V")} + + action := MsgToAction(msg) + + require.NotNil(t, action, "expected action for rebound key") + require.Equal(t, PRActionApprove, action.Type, "expected approve action for rebound key") +} diff --git a/internal/tui/components/prview/prview.go b/internal/tui/components/prview/prview.go index a374073b4..28cc1a165 100644 --- a/internal/tui/components/prview/prview.go +++ b/internal/tui/components/prview/prview.go @@ -178,12 +178,9 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { switch { case key.Matches(msg, keys.PRKeys.PrevSidebarTab): m.carousel.MoveLeft() - return m, nil case key.Matches(msg, keys.PRKeys.NextSidebarTab): m.carousel.MoveRight() - return m, nil } - return m, nil } } @@ -259,22 +256,12 @@ func (m Model) View() string { } func (m *Model) renderFullNameAndNumber() string { - return lipgloss.NewStyle(). - PaddingLeft(1). - Width(m.width). - Background(m.ctx.Theme.SelectedBackground). - Foreground(m.ctx.Theme.SecondaryText). - Render(fmt.Sprintf("%s · #%d", m.pr.Data.Primary.GetRepoNameWithOwner(), m.pr.Data.Primary.GetNumber())) + return common.RenderPreviewHeader(m.ctx.Theme, m.width, + fmt.Sprintf("%s · #%d", m.pr.Data.Primary.GetRepoNameWithOwner(), m.pr.Data.Primary.GetNumber())) } func (m *Model) renderTitle() string { - return lipgloss.NewStyle().Height(3).Width(m.width).Background( - m.ctx.Theme.SelectedBackground).PaddingLeft(1).Render( - lipgloss.PlaceVertical(3, lipgloss.Center, m.ctx.Styles.Common.MainTextStyle. - Background(m.ctx.Theme.SelectedBackground). - Render(m.pr.Data.Primary.Title), - ), - ) + return common.RenderPreviewTitle(m.ctx.Theme, m.ctx.Styles.Common, m.width, m.pr.Data.Primary.Title) } func (m *Model) renderBranches() string { @@ -744,6 +731,14 @@ func (m *Model) GoToFirstTab() { m.carousel.SetCursor(0) } +func (m *Model) GoToActivityTab() { + m.carousel.SetCursor(1) // Activity is the second tab (index 1) +} + +func (m Model) SelectedTab() string { + return m.carousel.SelectedItem() +} + func (m *Model) SetSummaryViewMore() { m.summaryViewMore = true } diff --git a/internal/tui/components/sidebar/sidebar.go b/internal/tui/components/sidebar/sidebar.go index cb26dd094..f6c8fcfc8 100644 --- a/internal/tui/components/sidebar/sidebar.go +++ b/internal/tui/components/sidebar/sidebar.go @@ -93,6 +93,12 @@ func (m *Model) ScrollToBottom() { m.viewport.GotoBottom() } +func (m *Model) ScrollToPercent(percent float64) { + totalLines := m.viewport.TotalLineCount() + targetLine := int(float64(totalLines) * percent) + m.viewport.SetYOffset(targetLine) +} + func (m *Model) UpdateProgramContext(ctx *context.ProgramContext) { if ctx == nil { return diff --git a/internal/tui/components/table/table.go b/internal/tui/components/table/table.go index 9b102a3f0..dfd9b4460 100644 --- a/internal/tui/components/table/table.go +++ b/internal/tui/components/table/table.go @@ -2,6 +2,7 @@ package table import ( "fmt" + "strings" "time" "github.com/charmbracelet/bubbles/spinner" @@ -25,6 +26,7 @@ type Model struct { loadingSpinner spinner.Model dimensions constants.Dimensions rowsViewport listviewport.Model + ContentHeight int // Optional: override content height (0 = use default from config) } type Column struct { @@ -33,6 +35,7 @@ type Column struct { Width *int ComputedWidth int Grow *bool + Align *lipgloss.Position // Optional alignment (defaults to left) } type Row []string @@ -178,6 +181,17 @@ func (m *Model) SetRows(rows []Row) { m.SyncViewPortContent() } +// SetContentHeight sets a custom content height for rows (use 0 to use default config-based height) +func (m *Model) SetContentHeight(height int) { + m.ContentHeight = height + // Recalculate item height for viewport + itemHeight := height + if m.ctx.Config.Theme.Ui.Table.ShowSeparator { + itemHeight += 1 + } + m.rowsViewport.SetItemHeight(itemHeight) +} + func (m *Model) OnLineDown() { m.rowsViewport.NextItem() } @@ -210,10 +224,23 @@ func (m *Model) renderHeaderColumns() []string { } if column.Width != nil { - renderedColumns[i] = m.ctx.Styles.Table.TitleCellStyle. + title := column.Title + style := m.ctx.Styles.Table.TitleCellStyle + // Align headers for right-aligned columns + // Use padding-free style to avoid truncation from PlaceHorizontal + padding + if column.Align != nil && *column.Align == lipgloss.Right { + // Center short headers (icons), right-align longer headers (text) + headerAlign := lipgloss.Right + if lipgloss.Width(title) <= 2 { + headerAlign = lipgloss.Center + } + title = lipgloss.PlaceHorizontal(*column.Width, headerAlign, title) + style = lipgloss.NewStyle().Bold(true).Foreground(m.ctx.Theme.PrimaryText) + } + renderedColumns[i] = style. Width(*column.Width). MaxWidth(*column.Width). - Render(column.Title) + Render(title) takenWidth += *column.Width continue } @@ -293,16 +320,49 @@ func (m *Model) renderRow(rowId int, headerColumns []string) string { colWidth := lipgloss.Width(headerColumns[headerColId]) colHeight := 1 - if !m.ctx.Config.Theme.Ui.Table.Compact { + if m.ContentHeight > 0 { + // Use custom content height if set + colHeight = m.ContentHeight + } else if !m.ctx.Config.Theme.Ui.Table.Compact { colHeight = 2 } col := m.Rows[rowId][i] - renderedCol := style. - Width(colWidth). - MaxWidth(colWidth). - Height(colHeight). - MaxHeight(colHeight). - Render(col) + // For multi-line content, truncate long lines and pad short lines + // so lines don't wrap and background color extends properly + // Account for cell padding (1 left + 1 right = 2) + contentWidth := colWidth - 2 + if contentWidth < 1 { + contentWidth = 1 + } + var renderedCol string + if strings.Contains(col, "\n") { + // For multi-line content, apply cell style to each line individually + // This ensures background color extends properly despite ANSI resets in content + lines := strings.Split(col, "\n") + renderedLines := make([]string, len(lines)) + for j, line := range lines { + lineWidth := lipgloss.Width(line) + if lineWidth > contentWidth { + line = ansi.Truncate(line, contentWidth, constants.Ellipsis) + } + lineStyle := style.Width(colWidth).MaxWidth(colWidth).Height(1) + if column.Align != nil { + lineStyle = lineStyle.Align(*column.Align) + } + renderedLines[j] = lineStyle.Render(line) + } + renderedCol = strings.Join(renderedLines, "\n") + } else { + cellStyle := style. + Width(colWidth). + MaxWidth(colWidth). + Height(colHeight). + MaxHeight(colHeight) + if column.Align != nil { + cellStyle = cellStyle.Align(*column.Align) + } + renderedCol = cellStyle.Render(col) + } renderedColumns = append(renderedColumns, renderedCol) headerColId++ diff --git a/internal/tui/components/tabs/tabs.go b/internal/tui/components/tabs/tabs.go index d8ab9e510..044ec0304 100644 --- a/internal/tui/components/tabs/tabs.go +++ b/internal/tui/components/tabs/tabs.go @@ -128,7 +128,9 @@ func (m *Model) UpdateTabTitles() { title := cfg.Title // handle search section if i == 0 { - // noop + if title == "" { + title = constants.SearchIcon + } } else if tab.section.GetIsLoading() { title = fmt.Sprintf("%s %s", title, m.sectionTabs[i].spinner.View()) } else if m.ctx.Config.Theme.Ui.SectionsShowCount { diff --git a/internal/tui/constants/constants.go b/internal/tui/constants/constants.go index 7d2da793e..a0f6eb1b1 100644 --- a/internal/tui/constants/constants.go +++ b/internal/tui/constants/constants.go @@ -73,6 +73,13 @@ const ( UnknownRoleIcon = "󰭙" // \udb82\udf59 nf-md-account_question + // Notification type icons + WorkflowIcon = "" // \uf52e nf-oct-checklist (for CheckSuite/CI) + WorkflowRunIcon = "" // \uebd6 nf-cod-workflow (for CheckSuite default) + SecurityIcon = "󰒃" // \udb80\udc83 nf-md-shield_alert (for security alerts) + NotificationIcon = "" // \ueaa2 nf-cod-bell (generic notification fallback) + SearchIcon = "" // \uf002 nf-fa-search + Logo = `▜▔▚▐▔▌▚▔▐ ▌ ▟▁▞▐▔▌▁▚▐▔▌` ) diff --git a/internal/tui/context/context.go b/internal/tui/context/context.go index 6df8c986c..24fbcbd71 100644 --- a/internal/tui/context/context.go +++ b/internal/tui/context/context.go @@ -36,6 +36,7 @@ type ProgramContext struct { ScreenWidth int MainContentWidth int MainContentHeight int + SidebarOpen bool Config *config.Config ConfigFlag string Version string @@ -57,6 +58,10 @@ func (ctx *ProgramContext) GetViewSectionsConfig() []config.SectionConfig { Limit: utils.IntPtr(20), Type: &t, }.ToSectionConfig()) + case config.NotificationsView: + for _, cfg := range ctx.Config.NotificationsSections { + configs = append(configs, cfg.ToSectionConfig()) + } case config.PRsView: for _, cfg := range ctx.Config.PRSections { configs = append(configs, cfg.ToSectionConfig()) diff --git a/internal/tui/keys/issueKeys.go b/internal/tui/keys/issueKeys.go index 402224f30..496f268e6 100644 --- a/internal/tui/keys/issueKeys.go +++ b/internal/tui/keys/issueKeys.go @@ -47,7 +47,7 @@ var IssueKeys = IssueKeyMap{ ), ViewPRs: key.NewBinding( key.WithKeys("s"), - key.WithHelp("s", "switch to PRs"), + key.WithHelp("s", "switch to notifications"), ), } diff --git a/internal/tui/keys/keys.go b/internal/tui/keys/keys.go index f4ca3925d..0f101bbaf 100644 --- a/internal/tui/keys/keys.go +++ b/internal/tui/keys/keys.go @@ -10,6 +10,23 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/config" ) +// NotificationSubjectType indicates what type of content is being viewed in the notifications view +type NotificationSubjectType int + +const ( + NotificationSubjectNone NotificationSubjectType = iota + NotificationSubjectPR + NotificationSubjectIssue +) + +// notificationSubject tracks the current notification subject type for help display +var notificationSubject NotificationSubjectType + +// SetNotificationSubject sets the current notification subject type for help display +func SetNotificationSubject(subjectType NotificationSubjectType) { + notificationSubject = subjectType +} + type KeyMap struct { viewType config.ViewType Up key.Binding @@ -56,6 +73,17 @@ func (k KeyMap) FullHelp() [][]key.Binding { case config.RepoView: additionalKeys = BranchFullHelp() customKeys = append(customKeys, CustomBranchBindings...) + case config.NotificationsView: + additionalKeys = NotificationFullHelp() + // Include PR or Issue keys when viewing that subject type + switch notificationSubject { + case NotificationSubjectPR: + additionalKeys = append(additionalKeys, PRFullHelp()...) + customKeys = append(customKeys, CustomPRBindings...) + case NotificationSubjectIssue: + additionalKeys = append(additionalKeys, IssueFullHelp()...) + customKeys = append(customKeys, CustomIssueBindings...) + } default: additionalKeys = IssueFullHelp() customKeys = append(customKeys, CustomIssueBindings...) diff --git a/internal/tui/keys/keys_test.go b/internal/tui/keys/keys_test.go new file mode 100644 index 000000000..db990ac90 --- /dev/null +++ b/internal/tui/keys/keys_test.go @@ -0,0 +1,166 @@ +package keys + +import ( + "testing" + + "github.com/charmbracelet/bubbles/key" + + "github.com/dlvhdr/gh-dash/v4/internal/config" +) + +func TestSetNotificationSubject(t *testing.T) { + tests := []struct { + name string + subject NotificationSubjectType + expected NotificationSubjectType + }{ + {"none", NotificationSubjectNone, NotificationSubjectNone}, + {"pr", NotificationSubjectPR, NotificationSubjectPR}, + {"issue", NotificationSubjectIssue, NotificationSubjectIssue}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + SetNotificationSubject(tt.subject) + if notificationSubject != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, notificationSubject) + } + }) + } +} + +func TestFullHelpIncludesPRKeysForPRSubject(t *testing.T) { + // Set up for notifications view with PR subject + keymap := CreateKeyMapForView(config.NotificationsView) + SetNotificationSubject(NotificationSubjectPR) + + help := keymap.FullHelp() + + // Flatten all sections to check for keys + var allKeys []key.Binding + for _, section := range help { + allKeys = append(allKeys, section...) + } + + // Check that notification keys are present + found := findKeyByHelp(allKeys, "mark as done") + if !found { + t.Error("expected notification key 'mark as done' to be present") + } + + // Check that PR keys are present + found = findKeyByHelp(allKeys, "diff") + if !found { + t.Error("expected PR key 'diff' to be present when viewing PR notification") + } + + found = findKeyByHelp(allKeys, "approve") + if !found { + t.Error("expected PR key 'approve' to be present when viewing PR notification") + } + + // Clean up + SetNotificationSubject(NotificationSubjectNone) +} + +func TestFullHelpIncludesIssueKeysForIssueSubject(t *testing.T) { + // Set up for notifications view with Issue subject + keymap := CreateKeyMapForView(config.NotificationsView) + SetNotificationSubject(NotificationSubjectIssue) + + help := keymap.FullHelp() + + // Flatten all sections to check for keys + var allKeys []key.Binding + for _, section := range help { + allKeys = append(allKeys, section...) + } + + // Check that notification keys are present + found := findKeyByHelp(allKeys, "mark as done") + if !found { + t.Error("expected notification key 'mark as done' to be present") + } + + // Check that Issue keys are present + found = findKeyByHelp(allKeys, "label") + if !found { + t.Error("expected Issue key 'label' to be present when viewing Issue notification") + } + + found = findKeyByHelp(allKeys, "close") + if !found { + t.Error("expected Issue key 'close' to be present when viewing Issue notification") + } + + // Clean up + SetNotificationSubject(NotificationSubjectNone) +} + +func TestFullHelpExcludesPRKeysForNoSubject(t *testing.T) { + // Set up for notifications view with no subject + keymap := CreateKeyMapForView(config.NotificationsView) + SetNotificationSubject(NotificationSubjectNone) + + help := keymap.FullHelp() + + // Flatten all sections to check for keys + var allKeys []key.Binding + for _, section := range help { + allKeys = append(allKeys, section...) + } + + // Check that notification keys are present + found := findKeyByHelp(allKeys, "mark as done") + if !found { + t.Error("expected notification key 'mark as done' to be present") + } + + // Check that PR keys are NOT present + found = findKeyByHelp(allKeys, "diff") + if found { + t.Error("expected PR key 'diff' to NOT be present when no subject is selected") + } + + // Check that Issue keys are NOT present + found = findKeyByHelp(allKeys, "label") + if found { + t.Error("expected Issue key 'label' to NOT be present when no subject is selected") + } +} + +func TestFullHelpForPRViewDoesNotIncludeNotificationKeys(t *testing.T) { + // Set up for PR view (not notifications) + keymap := CreateKeyMapForView(config.PRsView) + SetNotificationSubject(NotificationSubjectNone) + + help := keymap.FullHelp() + + // Flatten all sections to check for keys + var allKeys []key.Binding + for _, section := range help { + allKeys = append(allKeys, section...) + } + + // Check that PR keys are present + found := findKeyByHelp(allKeys, "diff") + if !found { + t.Error("expected PR key 'diff' to be present in PR view") + } + + // Check that notification-specific keys are NOT present + found = findKeyByHelp(allKeys, "toggle bookmark") + if found { + t.Error("expected notification key 'toggle bookmark' to NOT be present in PR view") + } +} + +// findKeyByHelp searches for a key binding by its help description +func findKeyByHelp(keys []key.Binding, helpDesc string) bool { + for _, k := range keys { + if k.Help().Desc == helpDesc { + return true + } + } + return false +} diff --git a/internal/tui/keys/notificationKeys.go b/internal/tui/keys/notificationKeys.go new file mode 100644 index 000000000..0c79163ef --- /dev/null +++ b/internal/tui/keys/notificationKeys.go @@ -0,0 +1,82 @@ +package keys + +import ( + "github.com/charmbracelet/bubbles/key" +) + +type NotificationKeyMap struct { + View key.Binding + MarkAsDone key.Binding + MarkAllAsDone key.Binding + MarkAsRead key.Binding + MarkAllAsRead key.Binding + Unsubscribe key.Binding + ToggleBookmark key.Binding + Open key.Binding + SortByRepo key.Binding + SwitchToPRs key.Binding + ToggleSmartFiltering key.Binding +} + +var NotificationKeys = NotificationKeyMap{ + View: key.NewBinding( + key.WithKeys("enter"), + key.WithHelp("enter", "view notification"), + ), + MarkAsDone: key.NewBinding( + key.WithKeys("D"), + key.WithHelp("D", "mark as done"), + ), + MarkAllAsDone: key.NewBinding( + key.WithKeys("alt+d"), + key.WithHelp("Alt+d", "mark all as done"), + ), + MarkAsRead: key.NewBinding( + key.WithKeys("m"), + key.WithHelp("m", "mark as read"), + ), + MarkAllAsRead: key.NewBinding( + key.WithKeys("M"), + key.WithHelp("M", "mark all as read"), + ), + Unsubscribe: key.NewBinding( + key.WithKeys("u"), + key.WithHelp("u", "unsubscribe"), + ), + ToggleBookmark: key.NewBinding( + key.WithKeys("b"), + key.WithHelp("b", "toggle bookmark"), + ), + Open: key.NewBinding( + key.WithKeys("o"), + key.WithHelp("o", "open in browser"), + ), + SortByRepo: key.NewBinding( + key.WithKeys("S"), + key.WithHelp("S", "sort by repo"), + ), + SwitchToPRs: key.NewBinding( + key.WithKeys("s"), + key.WithHelp("s", "switch to PRs"), + ), + ToggleSmartFiltering: key.NewBinding( + key.WithKeys("t"), + key.WithHelp("t", "toggle smart filtering"), + ), +} + +func NotificationFullHelp() []key.Binding { + return []key.Binding{ + NotificationKeys.View, + NotificationKeys.MarkAsDone, + NotificationKeys.MarkAllAsDone, + NotificationKeys.MarkAsRead, + NotificationKeys.MarkAllAsRead, + NotificationKeys.Unsubscribe, + NotificationKeys.ToggleBookmark, + NotificationKeys.Open, + NotificationKeys.SortByRepo, + NotificationKeys.SwitchToPRs, + NotificationKeys.ToggleSmartFiltering, + } +} diff --git a/internal/tui/modelUtils.go b/internal/tui/modelUtils.go index 43f45c190..94fcde6e2 100644 --- a/internal/tui/modelUtils.go +++ b/internal/tui/modelUtils.go @@ -168,6 +168,7 @@ func (m *Model) runCustomPRCommand(commandTemplate string, prData *prrow.Data) t "PrNumber": prData.Primary.Number, "HeadRefName": prData.Primary.HeadRefName, "BaseRefName": prData.Primary.BaseRefName, + "Author": prData.Primary.Author.Login, }) } @@ -194,6 +195,7 @@ func (m *Model) runCustomBranchCommand(commandTemplate string, branchData *prrow "PrNumber": branchData.Primary.Number, "HeadRefName": branchData.Primary.HeadRefName, "BaseRefName": branchData.Primary.BaseRefName, + "Author": branchData.Primary.Author.Login, }) } return m.runCustomCommand(commandTemplate, &input) diff --git a/internal/tui/theme/theme.go b/internal/tui/theme/theme.go index 1f70b79d1..fa7a0716f 100644 --- a/internal/tui/theme/theme.go +++ b/internal/tui/theme/theme.go @@ -19,6 +19,7 @@ type Theme struct { SuccessText lipgloss.AdaptiveColor // config.Theme.Colors.Text.Success WarningText lipgloss.AdaptiveColor // config.Theme.Colors.Text.Warning ErrorText lipgloss.AdaptiveColor // config.Theme.Colors.Text.Error + ActorText lipgloss.AdaptiveColor // config.Theme.Colors.Text.Actor NewContributorIconColor lipgloss.AdaptiveColor // config.Theme.Colors.Icon.NewContributor ContributorIconColor lipgloss.AdaptiveColor // config.Theme.Colors.Icon.Contributor CollaboratorIconColor lipgloss.AdaptiveColor // config.Theme.Colors.Icon.Collaborator @@ -45,6 +46,7 @@ var DefaultTheme = &Theme{ SuccessText: lipgloss.AdaptiveColor{Light: "002", Dark: "002"}, WarningText: lipgloss.AdaptiveColor{Light: "003", Dark: "003"}, ErrorText: lipgloss.AdaptiveColor{Light: "001", Dark: "001"}, + ActorText: lipgloss.AdaptiveColor{Light: "244", Dark: "251"}, // Same as SecondaryText NewContributorIconColor: lipgloss.AdaptiveColor{Light: "077", Dark: "077"}, ContributorIconColor: lipgloss.AdaptiveColor{Light: "075", Dark: "075"}, CollaboratorIconColor: lipgloss.AdaptiveColor{Light: "178", Dark: "178"}, @@ -118,6 +120,10 @@ func ParseTheme(cfg *config.Config) Theme { cfg.Theme.Colors.Inline.Text.Error, DefaultTheme.ErrorText, ) + DefaultTheme.ActorText = _shimHex( + cfg.Theme.Colors.Inline.Text.Actor, + DefaultTheme.ActorText, + ) DefaultTheme.NewContributorIconColor = _shimHex( cfg.Theme.Colors.Inline.Icon.NewContributor, DefaultTheme.NewContributorIconColor, diff --git a/internal/tui/ui.go b/internal/tui/ui.go index ae8e051a8..65c5d5d2b 100644 --- a/internal/tui/ui.go +++ b/internal/tui/ui.go @@ -27,6 +27,9 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/components/footer" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/issuessection" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/issueview" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/notificationrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/notificationssection" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/notificationview" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prssection" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prview" @@ -41,20 +44,22 @@ import ( ) type Model struct { - keys *keys.KeyMap - sidebar sidebar.Model - prView prview.Model - issueSidebar issueview.Model - branchSidebar branchsidebar.Model - currSectionId int - footer footer.Model - repo section.Section - prs []section.Section - issues []section.Section - tabs tabs.Model - ctx *context.ProgramContext - taskSpinner spinner.Model - tasks map[string]context.Task + keys *keys.KeyMap + sidebar sidebar.Model + prView prview.Model + issueSidebar issueview.Model + branchSidebar branchsidebar.Model + notificationView notificationview.Model + currSectionId int + footer footer.Model + repo section.Section + prs []section.Section + issues []section.Section + notifications []section.Section + tabs tabs.Model + ctx *context.ProgramContext + taskSpinner spinner.Model + tasks map[string]context.Task } func NewModel(location config.Location) Model { @@ -92,6 +97,7 @@ func NewModel(location config.Location) Model { m.prView = prview.NewModel(m.ctx) m.issueSidebar = issueview.NewModel(m.ctx) m.branchSidebar = branchsidebar.NewModel(m.ctx) + m.notificationView = notificationview.NewModel(m.ctx) m.tabs = tabs.NewModel(m.ctx) return m @@ -186,7 +192,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } if m.issueSidebar.IsTextInputBoxFocused() { - m.issueSidebar, cmd = m.issueSidebar.Update(msg) + m.issueSidebar, cmd, _ = m.issueSidebar.Update(msg) m.syncSidebar() return m, cmd } @@ -370,73 +376,44 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { cmds = append(cmds, m.openBrowser()) case key.Matches(msg, keys.PRKeys.Approve): - m.prView.GoToFirstTab() - m.sidebar.IsOpen = true - cmd = m.prView.SetIsApproving(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForPRInput(m.prView.SetIsApproving) case key.Matches(msg, keys.PRKeys.Assign): - m.prView.GoToFirstTab() - m.sidebar.IsOpen = true - cmd = m.prView.SetIsAssigning(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForPRInput(m.prView.SetIsAssigning) case key.Matches(msg, keys.PRKeys.Unassign): - m.prView.GoToFirstTab() - m.sidebar.IsOpen = true - cmd = m.prView.SetIsUnassigning(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForPRInput(m.prView.SetIsUnassigning) case key.Matches(msg, keys.PRKeys.Comment): - m.prView.GoToFirstTab() - m.sidebar.IsOpen = true - cmd = m.prView.SetIsCommenting(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForPRInput(m.prView.SetIsCommenting) case key.Matches(msg, keys.PRKeys.Close): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("close") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "close") } return m, cmd case key.Matches(msg, keys.PRKeys.Ready): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("ready") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "ready") } return m, cmd case key.Matches(msg, keys.PRKeys.Reopen): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("reopen") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "reopen") } return m, cmd case key.Matches(msg, keys.PRKeys.Merge): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("merge") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "merge") } return m, cmd case key.Matches(msg, keys.PRKeys.Update): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("update") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "update") } return m, cmd @@ -466,48 +443,26 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { cmds = append(cmds, m.openBrowser()) case key.Matches(msg, keys.IssueKeys.Label): - m.sidebar.IsOpen = true - cmd = m.issueSidebar.SetIsLabeling(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForInput(m.issueSidebar.SetIsLabeling) case key.Matches(msg, keys.IssueKeys.Assign): - m.sidebar.IsOpen = true - cmd = m.issueSidebar.SetIsAssigning(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForInput(m.issueSidebar.SetIsAssigning) case key.Matches(msg, keys.IssueKeys.Unassign): - m.sidebar.IsOpen = true - cmd = m.issueSidebar.SetIsUnassigning(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForInput(m.issueSidebar.SetIsUnassigning) case key.Matches(msg, keys.IssueKeys.Comment): - m.sidebar.IsOpen = true - cmd = m.issueSidebar.SetIsCommenting(true) - m.syncMainContentWidth() - m.syncSidebar() - m.sidebar.ScrollToBottom() - return m, cmd + return m, m.openSidebarForInput(m.issueSidebar.SetIsCommenting) case key.Matches(msg, keys.IssueKeys.Close): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("close") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "close") } return m, cmd case key.Matches(msg, keys.IssueKeys.Reopen): - if currRowData != nil && currSection != nil { - currSection.SetPromptConfirmationAction("reopen") - cmd = currSection.SetIsPromptConfirmationShown(true) + if currRowData != nil { + cmd = m.promptConfirmation(currSection, "reopen") } return m, cmd @@ -516,6 +471,146 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.syncMainContentWidth() m.setCurrSectionId(m.getCurrentViewDefaultSection()) + currSections := m.getCurrentViewSections() + if len(currSections) == 0 { + newSections, fetchSectionsCmds := m.fetchAllViewSections() + currSections = newSections + cmds = append(cmds, m.tabs.SetAllLoading()...) + cmd = fetchSectionsCmds + } + m.setCurrentViewSections(currSections) + cmds = append(cmds, m.onViewedRowChanged()) + } + case m.ctx.View == config.NotificationsView: + switch { + case key.Matches(msg, m.keys.OpenGithub): + cmds = append(cmds, m.openBrowser()) + + // PR keybindings when viewing a PR notification + case m.notificationView.GetSubjectPR() != nil: + var prCmd tea.Cmd + m.prView, prCmd = m.prView.Update(msg) + + if !m.prView.IsTextInputBoxFocused() { + action := prview.MsgToAction(msg) + if action != nil { + switch action.Type { + case prview.PRActionApprove: + return m, m.openSidebarForPRInput(m.prView.SetIsApproving) + + case prview.PRActionAssign: + return m, m.openSidebarForPRInput(m.prView.SetIsAssigning) + + case prview.PRActionUnassign: + return m, m.openSidebarForPRInput(m.prView.SetIsUnassigning) + + case prview.PRActionComment: + return m, m.openSidebarForPRInput(m.prView.SetIsCommenting) + + case prview.PRActionDiff: + if pr := m.notificationView.GetSubjectPR(); pr != nil { + cmd = common.DiffPR(pr.GetNumber(), pr.GetRepoNameWithOwner(), m.ctx.Config.GetFullScreenDiffPagerEnv()) + } + return m, cmd + + case prview.PRActionCheckout: + if pr := m.notificationView.GetSubjectPR(); pr != nil { + cmd, _ = notificationssection.CheckoutPR(m.ctx, pr.GetNumber(), pr.GetRepoNameWithOwner()) + } + return m, cmd + + case prview.PRActionClose: + return m, m.promptConfirmation(currSection, "close") + + case prview.PRActionReady: + return m, m.promptConfirmation(currSection, "ready") + + case prview.PRActionReopen: + return m, m.promptConfirmation(currSection, "reopen") + + case prview.PRActionMerge: + return m, m.promptConfirmation(currSection, "merge") + + case prview.PRActionUpdate: + return m, m.promptConfirmation(currSection, "update") + + case prview.PRActionSummaryViewMore: + m.prView.SetSummaryViewMore() + m.syncSidebar() + return m, nil + } + } + } + + // Always sync and return after updating prView - needed for tab navigation + // which updates carousel state but doesn't return a command + m.syncSidebar() + return m, prCmd + + // Issue keybindings when viewing an Issue notification + case m.notificationView.GetSubjectIssue() != nil: + var issueCmd tea.Cmd + var action *issueview.IssueAction + m.issueSidebar, issueCmd, action = m.issueSidebar.Update(msg) + + if action != nil { + switch action.Type { + case issueview.IssueActionLabel: + return m, m.openSidebarForInput(m.issueSidebar.SetIsLabeling) + + case issueview.IssueActionAssign: + return m, m.openSidebarForInput(m.issueSidebar.SetIsAssigning) + + case issueview.IssueActionUnassign: + return m, m.openSidebarForInput(m.issueSidebar.SetIsUnassigning) + + case issueview.IssueActionComment: + return m, m.openSidebarForInput(m.issueSidebar.SetIsCommenting) + + case issueview.IssueActionClose: + return m, m.promptConfirmation(currSection, "close") + + case issueview.IssueActionReopen: + return m, m.promptConfirmation(currSection, "reopen") + } + } + + if issueCmd != nil { + m.syncSidebar() + return m, issueCmd + } + + // Notification-specific keybindings + case key.Matches(msg, keys.NotificationKeys.View): + // View notification content and mark as read + cmds = append(cmds, m.loadNotificationContent()) + + case key.Matches(msg, keys.NotificationKeys.MarkAsDone): + // Already handled in the section's Update method + cmd = m.updateSection(currSection.GetId(), currSection.GetType(), msg) + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.MarkAllAsDone): + if currSection != nil { + currSection.SetPromptConfirmationAction("done_all") + cmd = currSection.SetIsPromptConfirmationShown(true) + } + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.Open): + // Handled in the section's Update method + cmd = m.updateSection(currSection.GetId(), currSection.GetType(), msg) + return m, cmd + + case key.Matches(msg, keys.NotificationKeys.SortByRepo): + cmd = m.updateSection(currSection.GetId(), currSection.GetType(), msg) + return m, cmd + + case key.Matches(msg, keys.PRKeys.ViewIssues): + m.ctx.View = m.switchSelectedView() + m.syncMainContentWidth() + m.setCurrSectionId(m.getCurrentViewDefaultSection()) + currSections := m.getCurrentViewSections() if len(currSections) == 0 { newSections, fetchSectionsCmds := m.fetchAllViewSections() @@ -588,6 +683,64 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { log.Error("failed enriching pr", "err", msg.Err) } + case notificationPRFetchedMsg: + if msg.Err == nil { + // Convert enriched PR to prrow.Data for display + prData := msg.PR.ToPullRequestData() + m.notificationView.SetSubjectPR(&prrow.Data{ + Primary: &prData, + Enriched: msg.PR, + IsEnriched: true, + }, msg.NotificationId) + keys.SetNotificationSubject(keys.NotificationSubjectPR) + // Update sidebar with PR view + width := m.sidebar.GetSidebarContentWidth() + m.prView.SetSectionId(0) + m.prView.SetRow(m.notificationView.GetSubjectPR()) + m.prView.SetWidth(width) + m.prView.SetEnrichedPR(msg.PR) + // Switch to Activity tab and scroll to bottom if there's a latest comment + // (indicates there's new activity to show) + if msg.LatestCommentUrl != "" { + m.prView.GoToActivityTab() + m.sidebar.SetContent(m.prView.View()) + m.sidebar.ScrollToBottom() + } else { + // For notifications without comments (new PRs, state changes, etc.) + // show the Overview tab without scrolling + m.prView.GoToFirstTab() + m.sidebar.SetContent(m.prView.View()) + } + m.markNotificationAsRead(msg.NotificationId) + } else { + log.Error("failed fetching notification PR", "err", msg.Err) + } + + case notificationIssueFetchedMsg: + if msg.Err == nil { + m.notificationView.SetSubjectIssue(&msg.Issue, msg.NotificationId) + keys.SetNotificationSubject(keys.NotificationSubjectIssue) + // Update sidebar with Issue view + width := m.sidebar.GetSidebarContentWidth() + m.issueSidebar.SetSectionId(0) + m.issueSidebar.SetRow(m.notificationView.GetSubjectIssue()) + m.issueSidebar.SetWidth(width) + m.sidebar.SetContent(m.issueSidebar.View()) + // Scroll to bottom if there's a latest comment (indicates new activity) + if msg.LatestCommentUrl != "" { + m.sidebar.ScrollToBottom() + } + m.markNotificationAsRead(msg.NotificationId) + } else { + log.Error("failed fetching notification Issue", "err", msg.Err) + } + + case notificationssection.UpdateNotificationReadStateMsg: + m.updateNotificationSections(msg) + + case notificationssection.UpdateNotificationCommentsMsg: + cmds = append(cmds, m.updateNotificationSections(msg)) + case spinner.TickMsg: if len(m.tasks) > 0 { taskSpinner, internalTickCmd := m.taskSpinner.Update(msg) @@ -654,7 +807,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } if m.issueSidebar.IsTextInputBoxFocused() { - m.issueSidebar, issueSidebarCmd = m.issueSidebar.Update(msg) + m.issueSidebar, issueSidebarCmd, _ = m.issueSidebar.Update(msg) m.syncSidebar() } @@ -730,18 +883,53 @@ type initMsg struct { RepoUrl string } +// Message types for notification subject fetching +type notificationPRFetchedMsg struct { + NotificationId string + PR data.EnrichedPullRequestData + LatestCommentUrl string + Err error +} + +type notificationIssueFetchedMsg struct { + NotificationId string + Issue data.IssueData + LatestCommentUrl string + Err error +} + func (m *Model) setCurrSectionId(newSectionId int) { m.currSectionId = newSectionId m.tabs.SetCurrSectionId(newSectionId) } +func (m *Model) updateNotificationSections(msg tea.Msg) tea.Cmd { + var cmds []tea.Cmd + for i := range m.notifications { + if m.notifications[i] != nil { + var cmd tea.Cmd + m.notifications[i], cmd = m.notifications[i].Update(msg) + cmds = append(cmds, cmd) + } + } + return tea.Batch(cmds...) +} + +func (m *Model) markNotificationAsRead(notificationId string) { + readStateMsg := notificationssection.UpdateNotificationReadStateMsg{ + Id: notificationId, + Unread: false, + } + m.updateNotificationSections(readStateMsg) +} + func (m *Model) onViewedRowChanged() tea.Cmd { m.prView.SetSummaryViewLess() m.prView.GoToFirstTab() - m.syncSidebar() - cmd := m.prView.EnrichCurrRow() + sidebarCmd := m.syncSidebar() + enrichCmd := m.prView.EnrichCurrRow() m.sidebar.ScrollToTop() - return cmd + return tea.Batch(sidebarCmd, enrichCmd) } func (m *Model) onWindowSizeChanged(msg tea.WindowSizeMsg) { @@ -767,6 +955,7 @@ func (m *Model) syncProgramContext() { m.prView.UpdateProgramContext(m.ctx) m.issueSidebar.UpdateProgramContext(m.ctx) m.branchSidebar.UpdateProgramContext(m.ctx) + m.notificationView.UpdateProgramContext(m.ctx) } func (m *Model) updateSection(id int, sType string, msg tea.Msg) (cmd tea.Cmd) { @@ -775,6 +964,11 @@ func (m *Model) updateSection(id int, sType string, msg tea.Msg) (cmd tea.Cmd) { case reposection.SectionType: m.repo, cmd = m.repo.Update(msg) + case notificationssection.SectionType: + if id < len(m.notifications) && m.notifications[id] != nil { + m.notifications[id], cmd = m.notifications[id].Update(msg) + } + case prssection.SectionType: updatedSection, cmd = m.prs[id].Update(msg) m.prs[id] = updatedSection @@ -811,6 +1005,29 @@ func (m *Model) syncMainContentWidth() { sideBarOffset = m.ctx.Config.Defaults.Preview.Width } m.ctx.MainContentWidth = m.ctx.ScreenWidth - sideBarOffset + m.ctx.SidebarOpen = m.sidebar.IsOpen +} + +func (m *Model) openSidebarForPRInput(setFunc func(bool) tea.Cmd) tea.Cmd { + m.prView.GoToFirstTab() + return m.openSidebarForInput(setFunc) +} + +func (m *Model) openSidebarForInput(setFunc func(bool) tea.Cmd) tea.Cmd { + m.sidebar.IsOpen = true + cmd := setFunc(true) + m.syncMainContentWidth() + m.syncSidebar() + m.sidebar.ScrollToBottom() + return cmd +} + +func (m *Model) promptConfirmation(currSection section.Section, action string) tea.Cmd { + if currSection != nil { + currSection.SetPromptConfirmationAction(action) + return currSection.SetIsPromptConfirmationShown(true) + } + return nil } func (m *Model) syncSidebar() tea.Cmd { @@ -837,11 +1054,201 @@ func (m *Model) syncSidebar() tea.Cmd { m.issueSidebar.SetRow(row) m.issueSidebar.SetWidth(width) m.sidebar.SetContent(m.issueSidebar.View()) + case *notificationrow.Data: + notifId := row.GetId() + + // Check if we already have cached data for this notification (user already viewed it) + if m.notificationView.GetSubjectId() == notifId { + // Use cached data + if m.notificationView.GetSubjectPR() != nil { + m.prView.SetSectionId(0) + m.prView.SetRow(m.notificationView.GetSubjectPR()) + m.prView.SetWidth(width) + m.sidebar.SetContent(m.prView.View()) + } else if m.notificationView.GetSubjectIssue() != nil { + m.issueSidebar.SetSectionId(0) + m.issueSidebar.SetRow(m.notificationView.GetSubjectIssue()) + m.issueSidebar.SetWidth(width) + m.sidebar.SetContent(m.issueSidebar.View()) + } + return nil + } + + // Show prompt to view notification (don't auto-fetch) + // User must press Enter to view content and mark as read + m.sidebar.SetContent(m.renderNotificationPrompt(row, width)) } return cmd } +func (m *Model) renderNotificationPrompt(row *notificationrow.Data, width int) string { + var content strings.Builder + + subjectType := row.GetSubjectType() + leftMargin := " " // Left margin for content + + // Styles + normalText := lipgloss.NewStyle().Foreground(m.ctx.Theme.PrimaryText) + faintText := lipgloss.NewStyle().Foreground(m.ctx.Theme.FaintText) + // Highlighted key style for main prompt (with background) + highlightKeyStyle := lipgloss.NewStyle(). + Foreground(m.ctx.Theme.PrimaryText). + Background(m.ctx.Theme.FaintBorder). + Padding(0, 1) + // Simple key style for table (no background) + keyStyle := lipgloss.NewStyle(). + Foreground(m.ctx.Theme.PrimaryText) + actionStyle := lipgloss.NewStyle().Foreground(m.ctx.Theme.SuccessText) + headerStyle := lipgloss.NewStyle(). + Foreground(m.ctx.Theme.PrimaryText). + Bold(true) + + // Determine subject type display name and primary action + typeName := "PR" + enterAction := "view" + if subjectType == "Issue" { + typeName = "Issue" + } else if subjectType != "PullRequest" { + typeName = subjectType + enterAction = "open in browser" + } + + // Main prompt: "Press Enter to view the PR" or "Press Enter to open in browser" + content.WriteString("\n") + content.WriteString(leftMargin) + content.WriteString(normalText.Render("Press ")) + content.WriteString(highlightKeyStyle.Render("Enter")) + if enterAction == "view" { + content.WriteString(normalText.Render(fmt.Sprintf(" to %s the %s", enterAction, typeName))) + } else { + content.WriteString(normalText.Render(fmt.Sprintf(" to %s", enterAction))) + } + content.WriteString("\n") + + // Note about marking as read + content.WriteString(leftMargin) + content.WriteString(faintText.Render("(Note: this will mark it as read)")) + content.WriteString("\n") + + content.WriteString("\n") + + // Other Actions header + content.WriteString(leftMargin) + content.WriteString(headerStyle.Render("Other Actions")) + content.WriteString("\n\n") + + // Key-action pairs (simple list without borders) + actions := []struct { + key string + action string + }{ + {"D", "mark as done"}, + {"m", "mark as read"}, + {"u", "unsubscribe"}, + {"b", "toggle bookmark"}, + {"t", "toggle filtering"}, + {"S", "sort by repo"}, + {"o", "open in browser"}, + } + + keyWidth := 7 // Width for key column + for _, a := range actions { + content.WriteString(leftMargin) + // Right-align the key in its column + padding := strings.Repeat(" ", keyWidth-len(a.key)) + content.WriteString(padding) + content.WriteString(keyStyle.Render(a.key)) + content.WriteString(" ") + content.WriteString(actionStyle.Render(a.action)) + content.WriteString("\n") + } + + // Add Enter at the end + content.WriteString(leftMargin) + padding := strings.Repeat(" ", keyWidth-len("Enter")) + content.WriteString(padding) + content.WriteString(keyStyle.Render("Enter")) + content.WriteString(" ") + content.WriteString(actionStyle.Render(enterAction)) + + return content.String() +} + +// loadNotificationContent fetches and displays notification content, marking it as read +func (m *Model) loadNotificationContent() tea.Cmd { + currRowData := m.getCurrRowData() + row, ok := currRowData.(*notificationrow.Data) + if !ok || row == nil { + return nil + } + + notifId := row.GetId() + subjectType := row.GetSubjectType() + subjectUrl := row.GetUrl() + latestCommentUrl := row.GetLatestCommentUrl() + + // Show loading indicator + width := m.sidebar.GetSidebarContentWidth() + m.notificationView.SetRow(row) + m.notificationView.SetWidth(width) + m.sidebar.SetContent(m.notificationView.View()) + + switch subjectType { + case "PullRequest": + return tea.Batch( + func() tea.Msg { + _ = data.MarkNotificationRead(notifId) + return notificationssection.UpdateNotificationReadStateMsg{ + Id: notifId, + Unread: false, + } + }, + func() tea.Msg { + pr, err := data.FetchPullRequest(subjectUrl) + return notificationPRFetchedMsg{ + NotificationId: notifId, + PR: pr, + LatestCommentUrl: latestCommentUrl, + Err: err, + } + }, + ) + case "Issue": + return tea.Batch( + func() tea.Msg { + _ = data.MarkNotificationRead(notifId) + return notificationssection.UpdateNotificationReadStateMsg{ + Id: notifId, + Unread: false, + } + }, + func() tea.Msg { + issue, err := data.FetchIssue(subjectUrl) + return notificationIssueFetchedMsg{ + NotificationId: notifId, + Issue: issue, + LatestCommentUrl: latestCommentUrl, + Err: err, + } + }, + ) + default: + // For discussions, releases, etc. - mark as read and open in browser + // since we can't show rich content for these types + return tea.Batch( + func() tea.Msg { + _ = data.MarkNotificationRead(notifId) + return notificationssection.UpdateNotificationReadStateMsg{ + Id: notifId, + Unread: false, + } + }, + m.openBrowser(), + ) + } +} + func (m *Model) fetchAllViewSections() ([]section.Section, tea.Cmd) { cmds := make([]tea.Cmd, 0) cmds = append(cmds, m.tabs.SetAllLoading()...) @@ -853,6 +1260,11 @@ func (m *Model) fetchAllViewSections() ([]section.Section, tea.Cmd) { cmds = append(cmds, cmd) m.repo = &s return nil, tea.Batch(cmds...) + case config.NotificationsView: + s, notifCmd := notificationssection.FetchAllSections(m.ctx, m.notifications) + cmds = append(cmds, notifCmd) + m.notifications = s + return s, tea.Batch(cmds...) case config.PRsView: s, prcmds := prssection.FetchAllSections(m.ctx, m.prs) cmds = append(cmds, prcmds) @@ -871,6 +1283,11 @@ func (m *Model) getCurrentViewSections() []section.Section { return []section.Section{} } return []section.Section{m.repo} + case config.NotificationsView: + if len(m.notifications) == 0 { + return []section.Section{} + } + return m.notifications case config.PRsView: return m.prs default: @@ -882,6 +1299,8 @@ func (m *Model) getCurrentViewDefaultSection() int { switch m.ctx.View { case config.RepoView: return 0 + case config.NotificationsView: + return 1 // First notification section after search section case config.PRsView: return 1 default: @@ -894,6 +1313,34 @@ func (m *Model) setCurrentViewSections(newSections []section.Section) { return } + // Handle notifications view with search section like PRs/Issues + if m.ctx.View == config.NotificationsView { + missingSearchSection := len(newSections) == 0 || (len(newSections) > 0 && newSections[0].GetId() != 0) + s := make([]section.Section, 0) + if missingSearchSection { + // Check if we have an existing search section to preserve + if len(m.notifications) > 0 && m.notifications[0] != nil && m.notifications[0].GetId() == 0 { + // Preserve existing search section with its filter state + s = append(s, m.notifications[0]) + } else { + // Create new search section only if none exists + search := notificationssection.NewModel( + 0, + m.ctx, + config.NotificationsSectionConfig{ + Title: "", + Filters: "archived:false", + }, + time.Now(), + ) + s = append(s, &search) + } + } + m.notifications = append(s, newSections...) + m.tabs.SetSections(m.notifications) + return + } + missingSearchSection := len(newSections) == 0 || (len(newSections) > 0 && newSections[0].GetId() != 0) s := make([]section.Section, 0) if m.ctx.View == config.PRsView { @@ -936,22 +1383,32 @@ func (m *Model) setCurrentViewSections(newSections []section.Section) { func (m *Model) switchSelectedView() config.ViewType { repoFF := config.IsFeatureEnabled(config.FF_REPO_VIEW) + // Reset notification subject when leaving notifications view + if m.ctx.View == config.NotificationsView { + keys.SetNotificationSubject(keys.NotificationSubjectNone) + } + + // View cycle: Notifications → PRs → Issues (→ Repo if enabled) → Notifications if repoFF { - switch true { - case m.ctx.View == config.RepoView: + switch m.ctx.View { + case config.NotificationsView: return config.PRsView - case m.ctx.View == config.PRsView: + case config.PRsView: return config.IssuesView - case m.ctx.View == config.IssuesView: + case config.IssuesView: return config.RepoView + case config.RepoView: + return config.NotificationsView } } - switch true { - case m.ctx.View == config.PRsView: + switch m.ctx.View { + case config.NotificationsView: + return config.PRsView + case config.PRsView: return config.IssuesView default: - return config.PRsView + return config.NotificationsView } } diff --git a/internal/tui/ui_test.go b/internal/tui/ui_test.go index cfc53dac2..08a54fca5 100644 --- a/internal/tui/ui_test.go +++ b/internal/tui/ui_test.go @@ -19,9 +19,14 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/config" "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prview" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/sidebar" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/keys" "github.com/dlvhdr/gh-dash/v4/internal/tui/markdown" "github.com/dlvhdr/gh-dash/v4/internal/tui/testutils" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) func TestFullOutput(t *testing.T) { @@ -155,3 +160,62 @@ func TestGetCurrentViewSections_RepoViewWithNilRepo(t *testing.T) { require.NotNil(t, sections, "sections should not be nil") require.Empty(t, sections, "sections should be empty when repo is nil") } + +func TestPromptConfirmation_NilSection(t *testing.T) { + // promptConfirmation should return nil when currSection is nil + m := Model{} + cmd := m.promptConfirmation(nil, "close") + require.Nil(t, cmd, "promptConfirmation should return nil when section is nil") +} + +func TestNotificationView_PRViewTabNavigation(t *testing.T) { + // This test verifies that tab navigation works in notification view when viewing a PR. + // Previously, the code only returned when prCmd != nil, but tab navigation + // (carousel.MoveLeft/MoveRight) doesn't return a command - it just updates state. + // The fix ensures we always sync sidebar and return after prView.Update(). + cfg, err := config.ParseConfig(config.Location{ + ConfigFlag: "../config/testdata/test-config.yml", + }) + require.NoError(t, err) + + ctx := &context.ProgramContext{ + Config: &cfg, + View: config.NotificationsView, + } + ctx.Theme = theme.ParseTheme(ctx.Config) + ctx.Styles = context.InitStyles(ctx.Theme) + + sidebarModel := sidebar.NewModel() + sidebarModel.UpdateProgramContext(ctx) + + m := Model{ + ctx: ctx, + keys: keys.Keys, + prView: prview.NewModel(ctx), + sidebar: sidebarModel, + } + + // Set up a PR notification subject so GetSubjectPR() returns non-nil + m.notificationView.SetSubjectPR(&prrow.Data{}, "test-notification-id") + + // Get initial tab + initialTab := m.prView.SelectedTab() + + // Send "next tab" key message + msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("]")} + newModel, _ := m.Update(msg) + m = newModel.(Model) + + // Verify tab changed + require.NotEqual(t, initialTab, m.prView.SelectedTab(), + "prView tab should have changed after pressing next tab key") + + // Now test going back + currentTab := m.prView.SelectedTab() + msg = tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("[")} + newModel, _ = m.Update(msg) + m = newModel.(Model) + + require.NotEqual(t, currentTab, m.prView.SelectedTab(), + "prView tab should have changed after pressing prev tab key") +} diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 95939357f..b49fcd11f 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -4,6 +4,8 @@ import ( "math" "strconv" "time" + + "github.com/charmbracelet/lipgloss" ) const ( @@ -94,3 +96,14 @@ func ShortNumber(n int) string { return strconv.Itoa(n/1000000) + "m" } + +// GetStylePrefix extracts ANSI codes from a lipgloss style without the trailing reset. +// This allows styled text to be concatenated without breaking parent background colors. +func GetStylePrefix(s lipgloss.Style) string { + rendered := s.Render("") + // Strip trailing reset sequence if present + if len(rendered) >= 4 && rendered[len(rendered)-4:] == "\x1b[0m" { + return rendered[:len(rendered)-4] + } + return rendered +} diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go new file mode 100644 index 000000000..5b893236e --- /dev/null +++ b/internal/utils/utils_test.go @@ -0,0 +1,140 @@ +package utils + +import ( + "testing" + + "github.com/charmbracelet/lipgloss" +) + +func TestGetStylePrefix(t *testing.T) { + tests := []struct { + name string + style lipgloss.Style + wantLen int + wantSame bool // whether input == output (no reset to strip) + }{ + { + name: "empty style returns empty or minimal ANSI", + style: lipgloss.NewStyle(), + wantLen: 0, + }, + { + name: "style with foreground color", + style: lipgloss.NewStyle().Foreground(lipgloss.Color("red")), + wantLen: 5, // At least some ANSI codes + }, + { + name: "style with background color", + style: lipgloss.NewStyle().Background(lipgloss.Color("blue")), + wantLen: 5, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetStylePrefix(tt.style) + // The result should not end with reset sequence + if len(result) >= 4 && result[len(result)-4:] == "\x1b[0m" { + t.Error("GetStylePrefix should strip trailing reset sequence") + } + }) + } +} + +func TestGetStylePrefix_StripsReset(t *testing.T) { + // Create a style that will produce a reset sequence + style := lipgloss.NewStyle().Foreground(lipgloss.Color("205")) + rendered := style.Render("") + + // Verify the raw render has a reset + hasReset := len(rendered) >= 4 && rendered[len(rendered)-4:] == "\x1b[0m" + + prefix := GetStylePrefix(style) + + // Prefix should not have reset even if rendered did + prefixHasReset := len(prefix) >= 4 && prefix[len(prefix)-4:] == "\x1b[0m" + if prefixHasReset { + t.Errorf("GetStylePrefix should strip reset, but got: %q", prefix) + } + + // If original had reset, prefix should be shorter + if hasReset && len(prefix) >= len(rendered) { + t.Error("GetStylePrefix should return shorter string when stripping reset") + } +} + +func TestGetStylePrefix_VariousStyles(t *testing.T) { + tests := []struct { + name string + style lipgloss.Style + }{ + { + name: "bold style", + style: lipgloss.NewStyle().Bold(true), + }, + { + name: "italic style", + style: lipgloss.NewStyle().Italic(true), + }, + { + name: "underline style", + style: lipgloss.NewStyle().Underline(true), + }, + { + name: "combined foreground and background", + style: lipgloss.NewStyle().Foreground(lipgloss.Color("red")).Background(lipgloss.Color("blue")), + }, + { + name: "adaptive color", + style: lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "#000000", Dark: "#ffffff"}), + }, + { + name: "256 color", + style: lipgloss.NewStyle().Foreground(lipgloss.Color("205")), + }, + { + name: "true color", + style: lipgloss.NewStyle().Foreground(lipgloss.Color("#ff5733")), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prefix := GetStylePrefix(tt.style) + + // Should not end with reset + if len(prefix) >= 4 && prefix[len(prefix)-4:] == "\x1b[0m" { + t.Errorf("GetStylePrefix should strip reset for %s, but got: %q", tt.name, prefix) + } + + // Should be valid ANSI (starts with escape if non-empty and has styling) + if len(prefix) > 0 && prefix[0] != '\x1b' { + // Only fail if the style would produce ANSI codes + rendered := tt.style.Render("") + if len(rendered) > 0 && rendered[0] == '\x1b' { + t.Errorf("GetStylePrefix for %s should start with escape, got: %q", tt.name, prefix) + } + } + }) + } +} + +func TestGetStylePrefix_ConcatenationPreservesStyle(t *testing.T) { + // Test that concatenating prefix + text works as expected + style := lipgloss.NewStyle().Foreground(lipgloss.Color("green")) + prefix := GetStylePrefix(style) + + // The prefix should be usable for concatenation + result := prefix + "Hello" + + // Result should contain the text + if len(result) < 5 { + t.Error("Result should contain 'Hello'") + } + + // Result should start with ANSI if style produces ANSI + rendered := style.Render("") + if len(rendered) > 0 && rendered[0] == '\x1b' && prefix[0] != '\x1b' { + t.Error("Prefix should start with ANSI escape when style has color") + } +}