diff --git a/api/queries_comments.go b/api/queries_comments.go index 8af17fd2ae6..bb1e9b2871c 100644 --- a/api/queries_comments.go +++ b/api/queries_comments.go @@ -129,7 +129,7 @@ func (c Comment) Identifier() string { } func (c Comment) AuthorLogin() string { - return c.Author.Login + return c.Author.DisplayName() } func (c Comment) Association() string { diff --git a/api/queries_issue.go b/api/queries_issue.go index 1a8e082ad8f..d545ef59f85 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -234,6 +234,11 @@ type Author struct { Login string } +// DisplayName returns a user-friendly name via actorDisplayName. +func (a Author) DisplayName() string { + return actorDisplayName("", a.Login, a.Name) +} + func (author Author) MarshalJSON() ([]byte, error) { if author.ID == "" { return json.Marshal(map[string]interface{}{ @@ -260,6 +265,11 @@ type CommentAuthor struct { // } `graphql:"... on User"` } +// DisplayName returns a user-friendly name via actorDisplayName. +func (a CommentAuthor) DisplayName() string { + return actorDisplayName("", a.Login, "") +} + // IssueCreate creates an issue in a GitHub repository func IssueCreate(client *Client, repo *Repository, params map[string]interface{}) (*Issue, error) { query := ` diff --git a/api/queries_pr_review.go b/api/queries_pr_review.go index a6fa34f9e88..b0a602bf4c9 100644 --- a/api/queries_pr_review.go +++ b/api/queries_pr_review.go @@ -52,7 +52,7 @@ func (prr PullRequestReview) Identifier() string { } func (prr PullRequestReview) AuthorLogin() string { - return prr.Author.Login + return prr.Author.DisplayName() } func (prr PullRequestReview) Association() string { @@ -143,6 +143,7 @@ type RequestedReviewer struct { const teamTypeName = "Team" const botTypeName = "Bot" +const userTypeName = "User" func (r RequestedReviewer) LoginOrSlug() string { if r.TypeName == teamTypeName { @@ -151,20 +152,13 @@ func (r RequestedReviewer) LoginOrSlug() string { return r.Login } -// DisplayName returns a user-friendly name for the reviewer. -// For Copilot bot, returns "Copilot (AI)". For teams, returns "org/slug". -// For users, returns "login (Name)" if name is available, otherwise just login. +// DisplayName returns a user-friendly name for the reviewer via actorDisplayName. +// Teams are handled separately as "org/slug". func (r RequestedReviewer) DisplayName() string { if r.TypeName == teamTypeName { return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug) } - if r.TypeName == botTypeName && r.Login == CopilotReviewerLogin { - return "Copilot (AI)" - } - if r.Name != "" { - return fmt.Sprintf("%s (%s)", r.Login, r.Name) - } - return r.Login + return actorDisplayName(r.TypeName, r.Login, r.Name) } func (r ReviewRequests) Logins() []string { @@ -221,10 +215,7 @@ func NewReviewerBot(login string) ReviewerBot { } func (b ReviewerBot) DisplayName() string { - if b.login == CopilotReviewerLogin { - return fmt.Sprintf("%s (AI)", CopilotActorName) - } - return b.Login() + return actorDisplayName("Bot", b.login, "") } func (r ReviewerBot) sealedReviewerCandidate() {} diff --git a/api/queries_repo.go b/api/queries_repo.go index d8ffa191dfc..d358255d8b9 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -147,6 +147,11 @@ type GitHubUser struct { DatabaseID int64 `json:"databaseId"` } +// DisplayName returns a user-friendly name via actorDisplayName. +func (u GitHubUser) DisplayName() string { + return actorDisplayName("", u.Login, u.Name) +} + // Actor is a superset of User and Bot, among others. // At the time of writing, some of these fields // are not directly supported by the Actor type and @@ -1087,6 +1092,24 @@ const CopilotAssigneeLogin = "copilot-swe-agent" const CopilotReviewerLogin = "copilot-pull-request-reviewer" const CopilotActorName = "Copilot" +// actorDisplayName returns a user-friendly display name for any actor. +// It handles bots (e.g. Copilot → "Copilot (AI)"), users with names +// ("login (Name)"), and falls back to just login. Empty typeName is +// treated as a possible bot or user — the login is checked against +// known bot logins first. +func actorDisplayName(typeName, login, name string) string { + if login == CopilotReviewerLogin || login == CopilotAssigneeLogin || login == CopilotActorName { + return fmt.Sprintf("%s (AI)", CopilotActorName) + } + if typeName == botTypeName { + return login + } + if name != "" { + return fmt.Sprintf("%s (%s)", login, name) + } + return login +} + type AssignableActor interface { DisplayName() string ID() string @@ -1110,12 +1133,9 @@ func NewAssignableUser(id, login, name string) AssignableUser { } } -// DisplayName returns a formatted string that uses Login and Name to be displayed e.g. 'Login (Name)' or 'Login' +// DisplayName returns a user-friendly name via actorDisplayName. func (u AssignableUser) DisplayName() string { - if u.name != "" { - return fmt.Sprintf("%s (%s)", u.login, u.name) - } - return u.login + return actorDisplayName(userTypeName, u.login, u.name) } func (u AssignableUser) ID() string { @@ -1145,10 +1165,7 @@ func NewAssignableBot(id, login string) AssignableBot { } func (b AssignableBot) DisplayName() string { - if b.login == CopilotAssigneeLogin { - return fmt.Sprintf("%s (AI)", CopilotActorName) - } - return b.Login() + return actorDisplayName(botTypeName, b.login, "") } func (b AssignableBot) ID() string { diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index ad0b8e8572a..ae00a98b217 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -563,6 +563,31 @@ func TestDisplayName(t *testing.T) { } } +func TestActorDisplayName(t *testing.T) { + tests := []struct { + name string + typeName string + login string + actName string + want string + }{ + {name: "copilot reviewer", typeName: "Bot", login: "copilot-pull-request-reviewer", want: "Copilot (AI)"}, + {name: "copilot assignee", typeName: "Bot", login: "copilot-swe-agent", want: "Copilot (AI)"}, + {name: "copilot without typename", typeName: "", login: "copilot-pull-request-reviewer", want: "Copilot (AI)"}, + {name: "copilot actor name login", typeName: "", login: "Copilot", want: "Copilot (AI)"}, + {name: "regular bot", typeName: "Bot", login: "dependabot", want: "dependabot"}, + {name: "user with name", typeName: "User", login: "octocat", actName: "Mona Lisa", want: "octocat (Mona Lisa)"}, + {name: "user without name", typeName: "User", login: "octocat", want: "octocat"}, + {name: "unknown type with name", typeName: "", login: "octocat", actName: "Mona Lisa", want: "octocat (Mona Lisa)"}, + {name: "empty login", typeName: "", login: "", want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, actorDisplayName(tt.typeName, tt.login, tt.actName)) + }) + } +} + func TestRepoExists(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index e41ad6acffe..5add5a71b1e 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -197,7 +197,7 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { // processing many issues with head and grep. fmt.Fprintf(out, "title:\t%s\n", issue.Title) fmt.Fprintf(out, "state:\t%s\n", issue.State) - fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login) + fmt.Fprintf(out, "author:\t%s\n", issue.Author.DisplayName()) fmt.Fprintf(out, "labels:\t%s\n", labels) fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount) fmt.Fprintf(out, "assignees:\t%s\n", assignees) @@ -222,7 +222,7 @@ func printHumanIssuePreview(opts *ViewOptions, baseRepo ghrepo.Interface, issue fmt.Fprintf(out, "%s • %s opened %s • %s\n", issueStateTitleWithColor(cs, issue), - issue.Author.Login, + issue.Author.DisplayName(), text.FuzzyAgo(opts.Now(), issue.CreatedAt), text.Pluralize(issue.Comments.TotalCount, "comment"), ) @@ -298,7 +298,7 @@ func issueAssigneeList(issue api.Issue) string { AssigneeNames := make([]string, 0, len(issue.Assignees.Nodes)) for _, assignee := range issue.Assignees.Nodes { - AssigneeNames = append(AssigneeNames, assignee.Login) + AssigneeNames = append(AssigneeNames, assignee.DisplayName()) } list := strings.Join(AssigneeNames, ", ") diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 564cce9132e..6e6859bc035 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -149,7 +149,7 @@ func printRawPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { fmt.Fprintf(out, "title:\t%s\n", pr.Title) fmt.Fprintf(out, "state:\t%s\n", prStateWithDraft(pr)) - fmt.Fprintf(out, "author:\t%s\n", pr.Author.Login) + fmt.Fprintf(out, "author:\t%s\n", pr.Author.DisplayName()) fmt.Fprintf(out, "labels:\t%s\n", labels) fmt.Fprintf(out, "assignees:\t%s\n", assignees) fmt.Fprintf(out, "reviewers:\t%s\n", reviewers) @@ -188,7 +188,7 @@ func printHumanPrPreview(opts *ViewOptions, baseRepo ghrepo.Interface, pr *api.P fmt.Fprintf(out, "%s • %s wants to merge %s into %s from %s • %s\n", shared.StateTitleWithColor(cs, *pr), - pr.Author.Login, + pr.Author.DisplayName(), text.Pluralize(pr.Commits.TotalCount, "commit"), pr.BaseRefName, pr.HeadRefName, @@ -351,7 +351,7 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { for _, review := range pr.Reviews.Nodes { if review.Author.Login != pr.Author.Login { - name := review.Author.Login + name := review.AuthorLogin() if name == "" { name = ghostName } @@ -364,7 +364,7 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { // Overwrite reviewer's state if a review request for the same reviewer exists. for _, reviewRequest := range pr.ReviewRequests.Nodes { - name := reviewRequest.RequestedReviewer.LoginOrSlug() + name := reviewRequest.RequestedReviewer.DisplayName() reviewerStates[name] = &reviewerState{ Name: name, State: requestedReviewState, @@ -406,7 +406,7 @@ func prAssigneeList(pr api.PullRequest) string { AssigneeNames := make([]string, 0, len(pr.Assignees.Nodes)) for _, assignee := range pr.Assignees.Nodes { - AssigneeNames = append(AssigneeNames, assignee.Login) + AssigneeNames = append(AssigneeNames, assignee.DisplayName()) } list := strings.Join(AssigneeNames, ", ")