`issue edit`, `pr edit`: handle display names in interactive assignee editing by BagToad · Pull Request #10990 · cli/cli

BagToad · 2025-05-15T17:20:19Z

This pull request introduces changes to support display names for both Bot and User assignee types in the interactive issue edit and pr edits flow.

- Refactored AssignedActors to return display names instead of logins. - Updated related functions to ensure consistency in display names. - Enhanced comments for clarity on display name logic and actor types.

…916-special-actor-assignee-names

andyfeller

How do these changes affect tests?

api/queries_issue.go

pkg/cmd/issue/edit/edit.go

pkg/cmd/pr/shared/editable.go

babakks

LGTM! But I'm not having the full context of the changes and still trying to catch up.

api/queries_issue.go

babakks · 2025-05-16T14:19:15Z

api/queries_issue.go

+		if a.TypeName == "User" {
+			u := NewAssignableUser(
+				a.ID,
+				a.Login,
+				a.Name,
+			)
+			displayNames = append(displayNames, u.DisplayName())
+		} else if a.TypeName == "Bot" {
+			b := NewAssignableBot(
+				a.ID,
+				a.Login,
+			)
+			displayNames = append(displayNames, b.DisplayName())
+		}


To future-proof this against new subtypes, do we need an else statement to grab whatever that is not User or Bot? I mean:

} else { displayNames = append(displayNames, a.Login) }

Personally I wouldn't want to do that because any other sub-types that get returned are unexpected behavior in my mind - we don't know what properties a future sub-type might come with - maybe there's a new sub-type that doesn't have an ID or a login. That's probably unlikely, but I'd still like to be intentional about when we decide to start supporting other actors and what unique behavior we may need to add to support them, just like Bot has.

Ah, hmm. I guess in this case the actor is already assigned 🤔

I don't know then. Maybe we could do something here. I'm unsure.

babakks · 2025-05-16T14:23:16Z

api/queries_repo.go

+}
+
 func (b AssignableBot) DisplayName() string {
+	if b.login == "copilot-swe-agent" {


Can't we use the same const for this and the one in your other PR (in CopilotReplacer)?

I mean, it makes sense to define it here in the api package and use it in individual command packages.

Yes, good call 👍

I'm going to do this in the final PR into trunk to avoid merge conflicts since this value is used in a few places.

andyfeller

Thanks for expanding the tests here! 💪 One question that doesn't block keeping this going, just curiosity.

andyfeller · 2025-05-16T15:58:27Z

pkg/cmd/issue/edit/edit_test.go

+				reg.Register(
+					httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
+					httpmock.GraphQLMutation(`
+					{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }`,


question: what is the importance of the empty __typename here?

I think the big reason was just to maintain consistency with the other mocks in this test suite.

For example:

func mockIssueUpdateLabels(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`mutation LabelAdd\b`), httpmock.GraphQLMutation(` { "data": { "addLabelsToLabelable": { "__typename": "" } } }`, func(inputs map[string]interface{}) {}), ) reg.Register( httpmock.GraphQL(`mutation LabelRemove\b`), httpmock.GraphQLMutation(` { "data": { "removeLabelsFromLabelable": { "__typename": "" } } }`, func(inputs map[string]interface{}) {}), ) }

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.72.0` -> `v2.73.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.73.0`](https://github.com/cli/cli/releases/tag/v2.73.0): GitHub CLI 2.73.0 [Compare Source](cli/cli@v2.72.0...v2.73.0) ####

Copilot Coding Agent Support You can now assign issues to GitHub Copilot directly from `gh`, just as you would assign them to a teammate. Use `gh issue edit <number> --add-assignee @copilot` to assign the GitHub Copilot coding agent, and Copilot will work in the background to understand the issue, propose a solution, and open a pull request when it's ready for your review. If you run `gh issue edit` interactively, `Copilot (AI)` will be displayed as a potential assignee. This feature is available for GitHub Copilot Pro+ and Copilot Enterprise subscribers. For more details, refer to [the full changelog post for Copilot coding agent](https://github.blog/changelog/2025-05-19-github-copilot-coding-agent-in-public-preview/). #### What's Changed ##### ✨ Features - Copilot is assignable to issues and pull requests with `issue edit` and `pr edit` by [@BagToad](https://github.com/BagToad) in cli/cli#10992 - `gh issue edit`: actors are assignable to issues by [@BagToad](https://github.com/BagToad) in cli/cli#10960 - `gh pr edit`: Assign actors to pull requests by [@BagToad](https://github.com/BagToad) in cli/cli#10984 - `issue edit`, `pr edit`: handle display names in interactive assignee editing by [@BagToad](https://github.com/BagToad) in cli/cli#10990 - `issue edit`, `pr edit`: Support special non-interactive (flags) assignee name `@copilot` by [@BagToad](https://github.com/BagToad) in cli/cli#10991 - \[gh issue/pr comment] Add support for last comment delete for issues and MRs by [@sinansonmez](https://github.com/sinansonmez) in cli/cli#10596 - \[gh issue view] Expose `closedByPullRequestsReferences` JSON field by [@iamazeem](https://github.com/iamazeem) in cli/cli#10941 - Accessible prompter always displays selection defaults in a format readable by a screen reader by [@BagToad](https://github.com/BagToad) in cli/cli#10937 ##### 🐛 Fixes - Fix `StatusJSONResponse` usage by [@babakks](https://github.com/babakks) in cli/cli#10810 - Fix panic on `gh pr view 0` by [@nopcoder](https://github.com/nopcoder) in cli/cli#10729 - Fix flakey test for accessible prompter by [@BagToad](https://github.com/BagToad) in cli/cli#10918 - Fix accessible prompter flaky tests by [@babakks](https://github.com/babakks) in cli/cli#10977 - Handle missing archive URLs on release download by [@williammartin](https://github.com/williammartin) in cli/cli#10947 - Fix bug when removing all MR reviewers by [@babakks](https://github.com/babakks) in cli/cli#10975 ##### 📚 Docs & Chores - Feature detect v1 projects on pr view by [@williammartin](https://github.com/williammartin) in cli/cli#10821 - Feature detect v1 projects on non-interactive pr create by [@williammartin](https://github.com/williammartin) in cli/cli#10909 - Feature detect v1 projects on web mode pr create by [@williammartin](https://github.com/williammartin) in cli/cli#10911 - Feature detect v1 projects on interactive `pr create` by [@williammartin](https://github.com/williammartin) in cli/cli#10915 - Feature detect v1 projects on pr edit by [@williammartin](https://github.com/williammartin) in cli/cli#10942 - Move predicate type filtering in `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#10670 - Improve assertion for disabled echo mode by [@babakks](https://github.com/babakks) in cli/cli#10927 #####

Dependencies - chore(deps): bump actions/attest-build-provenance from 2.2.2 to 2.3.0 by [@dependabot](https://github.com/dependabot) in cli/cli#10886 - chore(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.6 to 2.0.7 by [@dependabot](https://github.com/dependabot) in cli/cli#10869 #### What's Changed #### New Contributors - [@sinansonmez](https://github.com/sinansonmez) made their first contribution in cli/cli#10596 - [@nopcoder](https://github.com/nopcoder) made their first contribution in cli/cli#10729 **Full Changelog**: cli/cli@v2.72.0...v2.73.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

BagToad added 5 commits May 14, 2025 11:52

feat(pr edit): fetch assigned actors

71f22d8

feat(pr edit): fetch assignable actors

d4832ba

feat(editable): update assigned actors to use display names

eace1e8

- Refactored AssignedActors to return display names instead of logins. - Updated related functions to ensure consistency in display names. - Enhanced comments for clarity on display name logic and actor types.

fix(issues): fix non-interactive assignee matching to logins&IDs

9a5ea87

doc(pr edit): Add comments describing the use of DefaultLogins

52b6ebf

BagToad temporarily deployed to cli-automation May 15, 2025 17:20 — with GitHub Actions Inactive

Merge branch 'kw/gh-cli-909-911-assign-actors-to-prs' into kw/gh-cli-…

51b1e6c

…916-special-actor-assignee-names

BagToad marked this pull request as ready for review May 15, 2025 17:33

BagToad requested a review from a team as a code owner May 15, 2025 17:33

BagToad requested review from babakks and removed request for a team May 15, 2025 17:33

BagToad temporarily deployed to cli-automation May 15, 2025 17:33 — with GitHub Actions Inactive

cliAutomation assigned BagToad May 15, 2025

BagToad requested a review from williammartin May 15, 2025 18:41

Base automatically changed from kw/gh-cli-909-911-assign-actors-to-prs to kw/gh-cli-epic-900-actors-are-assignable May 15, 2025 20:22

andyfeller reviewed May 15, 2025

View reviewed changes

api/queries_issue.go Show resolved Hide resolved

pkg/cmd/issue/edit/edit.go Show resolved Hide resolved

pkg/cmd/pr/shared/editable.go Show resolved Hide resolved

babakks approved these changes May 16, 2025

View reviewed changes

test(editable): prompts use assignee display names

a22a1bb

andyfeller approved these changes May 16, 2025

View reviewed changes

BagToad merged commit 475163a into kw/gh-cli-epic-900-actors-are-assignable May 16, 2025
14 checks passed

BagToad deleted the kw/gh-cli-916-special-actor-assignee-names branch May 16, 2025 16:16

BagToad mentioned this pull request May 16, 2025

Copilot is assignable to issues and pull requests with issue edit and pr edit #10992

Merged

Conversation

BagToad commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

BagToad commented May 15, 2025 •

edited

Loading