Fix assignees being dropped from `gh pr edit` #11065

BagToad · 2025-06-03T13:28:11Z

Adds tests for both issue edit and pr edit even though pr edit is the broken one.

This commit is a handful of changes around `gh pr edit --add-assignee` behavior. Most notably, fixing a bug where the assigned actors weren't being dropped. In addition to this, I was refactoring the testing setup to allow for individual test table scenarios could be contained.

Copilot

Pull Request Overview

This PR ensures that assignees are preserved when running gh pr edit by tracking both display names and logins, and adds tests for both PR and issue editing workflows to cover actor-based assignments.

Populate Assignees.DefaultLogins in PR edit logic to preserve original assignees by login.
Refactor testSurveyor in PR edit tests and add a new interactive prompt test for display-name handling.
Update issue edit tests to assert on DefaultLogins and correct the mocked actor count.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
pkg/cmd/pr/edit/edit.go	Set `editable.Assignees.DefaultLogins` when actors are used
pkg/cmd/pr/edit/edit_test.go	Refactor `testSurveyor`, extend tests for actor display names
pkg/cmd/issue/edit/edit_test.go	Adjust tests to check `DefaultLogins` and correct mock data

Comments suppressed due to low confidence (2)

pkg/cmd/issue/edit/edit_test.go:809

There's a typo in the function name mockIsssueNumberGetWithAssignedActors (extra 's'). It should be mockIssueNumberGetWithAssignedActors.

func mockIsssueNumberGetWithAssignedActors(_ *testing.T, reg *httpmock.Registry,

pkg/cmd/pr/edit/edit_test.go:536

[nitpick] The fieldsToEdit closures are nearly identical across multiple test cases. Consider extracting a helper function or fixture to reduce duplication and make test changes easier to maintain.

Surveyor: testSurveyor{

Copilot · 2025-06-03T13:47:22Z

pkg/cmd/pr/edit/edit.go

@@ -228,6 +228,7 @@ func editRun(opts *EditOptions) error {
 	if pr.AssignedActorsUsed {
 		editable.Assignees.ActorAssignees = true
 		editable.Assignees.Default = pr.AssignedActors.DisplayNames()
+		editable.Assignees.DefaultLogins = pr.AssignedActors.Logins()
 	} else {
 		editable.Assignees.Default = pr.Assignees.Logins()


The DefaultLogins field is only set when AssignedActorsUsed is true. In the else branch, DefaultLogins remains nil, which could lead to missing login defaults in prompts. Consider also setting editable.Assignees.DefaultLogins = pr.Assignees.Logins() in the else block for consistency.

Suggested change

editable.Assignees.Default = pr.Assignees.Logins()

editable.Assignees.Default = pr.Assignees.Logins()

editable.Assignees.DefaultLogins = pr.Assignees.Logins()

I get it, but IDK man.

This is the kind of thing I meant when I said it was a bit "fragile". It's a bit "spooky action from a distance". It's hard to understand the relationship between these things. If you're up for it, we could spend a bit of time and see whether there is a design that we're happier with.

Yeah I think that'd be an interesting exercise.

BagToad · 2025-06-03T13:48:51Z

pkg/cmd/issue/edit/edit_test.go

 				EditFieldsSurvey: func(p prShared.EditPrompter, eo *prShared.Editable, _ string) error {
 					// Checking that the display name is being used in the prompt.
-					require.Equal(t, eo.Assignees.Default, []string{"hubot", "MonaLisa (Mona Display Name)"})
+					require.Equal(t, []string{"hubot"}, eo.Assignees.Default)
+					require.Equal(t, []string{"hubot"}, eo.Assignees.DefaultLogins)

-					// Mocking a selection of only MonaLisa in the prompt.
-					eo.Assignees.Value = []string{"MonaLisa (Mona Display Name)"}
+					// Adding MonaLisa as PR assignee, should preserve hubot.
+					eo.Assignees.Value = []string{"hubot", "MonaLisa (Mona Display Name)"}


Thought: We are missing out on testing that eo.Assignees.Default is properly loaded with display names and not logins.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | patch | `v2.74.1` -> `v2.74.2` | 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.74.2`](https://github.com/cli/cli/releases/tag/v2.74.2): GitHub CLI 2.74.2 [Compare Source](cli/cli@v2.74.1...v2.74.2) #### What's Changed ##### 🐛 Fixes - Fix assignees being dropped from `gh pr edit` by [@BagToad](https://github.com/BagToad) in cli/cli#11065 - Add accurate context when run rerun fails by [@leudz](https://github.com/leudz) in cli/cli#10774 - Avoid requesting MR reviewer twice by [@williammartin](https://github.com/williammartin) in cli/cli#11099 - Quote filenames suggested at the end of worklow run by [@williammartin](https://github.com/williammartin) in cli/cli#11134 - Fix expected error output of TestRepo/repo-rename-transfer-ownership by [@aconsuegra](https://github.com/aconsuegra) in cli/cli#10888 ##### 📚 Docs & Chores - Add instructions for MidnightBSD installation by [@laffer1](https://github.com/laffer1) in cli/cli#10699 - docs: update install command for Debian by [@MagneticNeedle](https://github.com/MagneticNeedle) in cli/cli#10935 - Fix step order for CodeQL workflow by [@BagToad](https://github.com/BagToad) in cli/cli#11145 - Add workflow to check `help wanted` labelling by [@williammartin](https://github.com/williammartin) in cli/cli#11105 - Quote workflow conditional by [@williammartin](https://github.com/williammartin) in cli/cli#11122 - Fix script path for help-wanted check by [@BagToad](https://github.com/BagToad) in cli/cli#11125 - Exclude 3rd party license compliance content from GHAS scanning by [@andyfeller](https://github.com/andyfeller) in cli/cli#11127 - Second fix for file not found in help-wanted check by [@BagToad](https://github.com/BagToad) in cli/cli#11128 - Ensure gh executes in workflow check script by [@williammartin](https://github.com/williammartin) in cli/cli#11133 - Improve help wanted check skipping logic by [@BagToad](https://github.com/BagToad) in cli/cli#11135 #####

Dependencies - Bump go to 1.24 by [@williammartin](https://github.com/williammartin) in cli/cli#11142 - chore(deps): bump mislav/bump-homebrew-formula-action from 3.2 to 3.4 by [@dependabot](https://github.com/dependabot) in cli/cli#11066 - chore(deps): bump github.com/sigstore/protobuf-specs from 0.4.2 to 0.4.3 by [@dependabot](https://github.com/dependabot) in cli/cli#11092 - chore(deps): bump google.golang.org/grpc from 1.72.0 to 1.72.2 by [@dependabot](https://github.com/dependabot) in cli/cli#11033 - chore(deps): bump actions/attest-build-provenance from 2.3.0 to 2.4.0 by [@dependabot](https://github.com/dependabot) in cli/cli#11107 - chore(deps): bump github.com/in-toto/attestation from 1.1.1 to 1.1.2 by [@dependabot](https://github.com/dependabot) in cli/cli#11123 - chore(deps): bump github.com/google/go-containerregistry from 0.20.3 to 0.20.6 by [@dependabot](https://github.com/dependabot) in cli/cli#11120 - Bump golangci-lint to v2 by [@williammartin](https://github.com/williammartin) in cli/cli#11121 #### New Contributors - [@MagneticNeedle](https://github.com/MagneticNeedle) made their first contribution in cli/cli#10935 - [@laffer1](https://github.com/laffer1) made their first contribution in cli/cli#10699 **Full Changelog**: cli/cli@v2.74.1...v2.74.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, 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).

andyfeller added 2 commits June 3, 2025 13:33

Fix test, remove partial standalone test

a24d39a

BagToad temporarily deployed to cli-automation June 3, 2025 13:28 — with GitHub Actions Inactive

test(pr): Add tests for actor assignees

ed4b901

BagToad force-pushed the andyfeller/gh-pr-edit-assignees branch from 1c896d8 to ed4b901 Compare June 3, 2025 13:30

BagToad marked this pull request as ready for review June 3, 2025 13:45

Copilot AI review requested due to automatic review settings June 3, 2025 13:45

BagToad requested a review from a team as a code owner June 3, 2025 13:45

BagToad requested a review from andyfeller June 3, 2025 13:45

BagToad temporarily deployed to cli-automation June 3, 2025 13:45 — with GitHub Actions Inactive

cliAutomation assigned BagToad Jun 3, 2025

Copilot AI reviewed Jun 3, 2025

View reviewed changes

BagToad commented Jun 3, 2025

View reviewed changes

williammartin approved these changes Jun 11, 2025

View reviewed changes

williammartin merged commit 8baf5cc into trunk Jun 11, 2025
17 checks passed

williammartin deleted the andyfeller/gh-pr-edit-assignees branch June 11, 2025 13:44

williammartin mentioned this pull request Jun 17, 2025

gh pr edit --add-assignee purges existing list #11140

Closed

This comment was marked as spam.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix assignees being dropped from `gh pr edit` #11065

Fix assignees being dropped from `gh pr edit` #11065

Uh oh!

BagToad commented Jun 3, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 3, 2025

Uh oh!

BagToad Jun 3, 2025

Uh oh!

williammartin Jun 3, 2025

Uh oh!

BagToad Jun 3, 2025

Uh oh!

BagToad Jun 3, 2025

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

	editable.Assignees.Default = pr.Assignees.Logins()
	editable.Assignees.Default = pr.Assignees.Logins()
	editable.Assignees.DefaultLogins = pr.Assignees.Logins()

Fix assignees being dropped from gh pr edit #11065

Fix assignees being dropped from gh pr edit #11065

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

BagToad Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

williammartin Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

BagToad Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

BagToad Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Fix assignees being dropped from `gh pr edit` #11065

Fix assignees being dropped from `gh pr edit` #11065

BagToad commented Jun 3, 2025 •

edited

Loading