diff --git a/api/queries_comments.go b/api/queries_comments.go index b0450c068..bb1e9b287 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 copilotDisplayName(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 1a8e082ad..d545ef59f 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 7f23f8b5e..48e758256 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 copilotDisplayName(prr.Author.Login) + return prr.Author.DisplayName() } func (prr PullRequestReview) Association() string { @@ -151,21 +151,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) } - displayName := copilotDisplayName(r.Login) - if displayName != r.Login { - return displayName - } - 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 { @@ -222,7 +214,7 @@ func NewReviewerBot(login string) ReviewerBot { } func (b ReviewerBot) DisplayName() string { - return copilotDisplayName(b.login) + return actorDisplayName("Bot", b.login, "") } func (r ReviewerBot) sealedReviewerCandidate() {} diff --git a/api/queries_repo.go b/api/queries_repo.go index 31dbf75f1..d4077eea9 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,13 +1092,21 @@ const CopilotAssigneeLogin = "copilot-swe-agent" const CopilotReviewerLogin = "copilot-pull-request-reviewer" const CopilotActorName = "Copilot" -// copilotDisplayName returns "Copilot (AI)" if the login is a known Copilot bot login, -// otherwise returns the login unchanged. Use this to translate raw bot logins into -// user-friendly display names in command output. -func copilotDisplayName(login string) string { - if login == CopilotReviewerLogin || login == CopilotAssigneeLogin { +// 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 } @@ -1120,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("User", u.login, u.name) } func (u AssignableUser) ID() string { @@ -1155,7 +1165,7 @@ func NewAssignableBot(id, login string) AssignableBot { } func (b AssignableBot) DisplayName() string { - return copilotDisplayName(b.login) + return actorDisplayName("Bot", b.login, "") } func (b AssignableBot) ID() string { diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 928a9e885..a45c26f8c 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -563,21 +563,29 @@ func TestDisplayName(t *testing.T) { } } -func TestCopilotDisplayName(t *testing.T) { +func TestActorDisplayName(t *testing.T) { tests := []struct { - login string - want string + name string + typeName string + login string + actName string + want string }{ - {login: "copilot-pull-request-reviewer", want: "Copilot (AI)"}, - {login: "copilot-swe-agent", want: "Copilot (AI)"}, - {login: "octocat", want: "octocat"}, - {login: "", want: ""}, + {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.login, func(t *testing.T) { - got := copilotDisplayName(tt.login) + t.Run(tt.name, func(t *testing.T) { + got := actorDisplayName(tt.typeName, tt.login, tt.actName) if got != tt.want { - t.Errorf("copilotDisplayName(%q) = %q, want %q", tt.login, got, tt.want) + t.Errorf("actorDisplayName(%q, %q, %q) = %q, want %q", tt.typeName, tt.login, tt.actName, got, tt.want) } }) } diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index e6ae63a10..6e6859bc0 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, @@ -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, ", ")