Skip to content

Commit 369ebb2

Browse files
author
Casey Quinn
authored
Fix repo resolution for PR helper commands (#34)
* fix(pr): honor repo override for review helpers * fix(pr): ignore base repo error for full refs
1 parent a213798 commit 369ebb2

File tree

9 files changed

+394
-93
lines changed

9 files changed

+394
-93
lines changed

README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@ Use `gh pr` subcommands to script review workflows without hand-writing REST or
1616

1717
```sh
1818
# list comments from the latest submitted review by the current user
19-
gh pr see-comments --org octo --repo demo --pr 42 --latest
19+
gh pr see-comments 42 --latest -R octo/demo
2020

2121
# reply to a specific review comment and auto-submit any pending review
22-
gh pr reply-comment --org octo --repo demo --pr 42 \
23-
--comment-id 123456789 --body "Thanks for catching that!" --auto-submit-pending
22+
gh pr reply-comment 42 --comment-id 123456789 \
23+
--body "Thanks for catching that!" --auto-submit-pending -R octo/demo
2424

2525
# GraphQL pending review flow (open → add inline thread → submit)
26-
gh pr review pending open --org octo --repo demo --pr 42
27-
gh pr review pending add --org octo --repo demo --pr 42 --review-id REVIEW_ID \
28-
--path src/main.go --line 87 --side RIGHT --body "nit: rename for clarity"
29-
gh pr review pending submit --org octo --repo demo --pr 42 --review-id REVIEW_ID --event COMMENT
26+
gh pr review pending open 42 -R octo/demo
27+
gh pr review pending add 42 --review-id REVIEW_ID \
28+
--path src/main.go --line 87 --side RIGHT --body "nit: rename for clarity" -R octo/demo
29+
gh pr review pending submit 42 --review-id REVIEW_ID --event COMMENT -R octo/demo
3030

3131
```
3232

pkg/cmd/pr/reply-comment/reply_comment.go

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import (
1010
"github.com/MakeNowJust/heredoc"
1111
"github.com/cli/cli/v2/api"
1212
"github.com/cli/cli/v2/internal/gh"
13+
"github.com/cli/cli/v2/internal/ghrepo"
1314
"github.com/cli/cli/v2/pkg/cmd/pr/reviewapi"
15+
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
1416
"github.com/cli/cli/v2/pkg/cmdutil"
1517
"github.com/cli/cli/v2/pkg/iostreams"
1618
"github.com/spf13/cobra"
@@ -23,8 +25,10 @@ type ReplyCommentOptions struct {
2325
HttpClient func() (*http.Client, error)
2426
Config func() (gh.Config, error)
2527

26-
Org string
27-
Repo string
28+
BaseRepo func() (ghrepo.Interface, error)
29+
30+
Repo ghrepo.Interface
31+
Selector string
2832
Pull int
2933
CommentID int64
3034
Body string
@@ -37,17 +41,26 @@ func NewCmdReplyComment(f *cmdutil.Factory) *cobra.Command {
3741
IO: f.IOStreams,
3842
HttpClient: f.HttpClient,
3943
Config: f.Config,
44+
BaseRepo: func() (ghrepo.Interface, error) {
45+
return f.BaseRepo()
46+
},
4047
}
4148

4249
cmd := &cobra.Command{
43-
Use: "reply-comment",
50+
Use: "reply-comment [<number> | <url> | <owner>/<repo>#<number>]",
4451
Short: "Reply to a pull request review comment",
4552
Long: heredoc.Doc(`
4653
Reply to an existing pull request review comment by comment identifier.
4754
`),
55+
Args: cobra.MaximumNArgs(1),
4856
RunE: func(cmd *cobra.Command, args []string) error {
57+
if len(args) > 0 {
58+
opts.Selector = args[0]
59+
}
4960
if opts.Pull <= 0 {
50-
return cmdutil.FlagErrorf("invalid value for --pr: %d", opts.Pull)
61+
if opts.Selector == "" {
62+
return cmdutil.FlagErrorf("must specify a pull request via --pr or as an argument")
63+
}
5164
}
5265
if opts.CommentID <= 0 {
5366
return cmdutil.FlagErrorf("invalid value for --comment-id: %d", opts.CommentID)
@@ -60,30 +73,34 @@ func NewCmdReplyComment(f *cmdutil.Factory) *cobra.Command {
6073
},
6174
}
6275

63-
cmd.Flags().StringVar(&opts.Org, "org", "", "Organization that owns the repository")
64-
cmd.Flags().StringVar(&opts.Repo, "repo", "", "Repository name")
6576
cmd.Flags().IntVar(&opts.Pull, "pr", 0, "Pull request number")
6677
cmd.Flags().Int64Var(&opts.CommentID, "comment-id", 0, "Review comment identifier to reply to")
6778
cmd.Flags().StringVar(&opts.Body, "body", "", "Reply text")
6879
cmd.Flags().BoolVar(&opts.AutoSubmitPending, "auto-submit-pending", false, "Submit pending reviews before replying")
6980
cmd.Flags().StringVar(&opts.Hostname, "hostname", "", "GitHub hostname (default to authenticated host)")
70-
71-
_ = cmd.MarkFlagRequired("org")
72-
_ = cmd.MarkFlagRequired("repo")
73-
_ = cmd.MarkFlagRequired("pr")
7481
_ = cmd.MarkFlagRequired("comment-id")
7582
_ = cmd.MarkFlagRequired("body")
7683

7784
return cmd
7885
}
7986

8087
func runReplyComment(ctx context.Context, opts *ReplyCommentOptions) error {
88+
repo, number, err := shared.ResolvePullRequest(opts.BaseRepo, opts.Selector, opts.Pull)
89+
if err != nil {
90+
return err
91+
}
92+
opts.Repo = repo
93+
opts.Pull = number
94+
8195
cfg, err := opts.Config()
8296
if err != nil {
8397
return err
8498
}
8599

86-
host, _ := cfg.Authentication().DefaultHost()
100+
host := repo.RepoHost()
101+
if host == "" {
102+
host, _ = cfg.Authentication().DefaultHost()
103+
}
87104
if opts.Hostname != "" {
88105
host = opts.Hostname
89106
}
@@ -94,34 +111,36 @@ func runReplyComment(ctx context.Context, opts *ReplyCommentOptions) error {
94111
}
95112

96113
service := reviewapi.NewService(httpClient, host)
114+
owner := repo.RepoOwner()
115+
name := repo.RepoName()
97116

98-
reply, err := service.ReplyToComment(ctx, opts.Org, opts.Repo, opts.Pull, opts.CommentID, opts.Body)
117+
reply, err := service.ReplyToComment(ctx, owner, name, opts.Pull, opts.CommentID, opts.Body)
99118
if err != nil {
100119
var pendingErr *reviewapi.PendingReviewError
101120
if errors.As(err, &pendingErr) {
102121
if !opts.AutoSubmitPending {
103-
return fmt.Errorf("pending review detected for %s/%s#%d. Submit the review or re-run with --auto-submit-pending, or use the GraphQL review thread mutation.", opts.Org, opts.Repo, opts.Pull)
122+
return fmt.Errorf("pending review detected for %s/%s#%d. Submit the review or re-run with --auto-submit-pending, or use the GraphQL review thread mutation.", owner, name, opts.Pull)
104123
}
105124

106125
reviewer, loginErr := service.CurrentLogin(ctx)
107126
if loginErr != nil {
108-
return formatReplyError(loginErr, opts, "failed to resolve authenticated user")
127+
return formatReplyError(loginErr, "failed to resolve authenticated user")
109128
}
110129

111-
submitted, submitErr := service.SubmitPendingReviews(ctx, opts.Org, opts.Repo, opts.Pull, reviewer, autoSubmitSummary)
130+
submitted, submitErr := service.SubmitPendingReviews(ctx, owner, name, opts.Pull, reviewer, autoSubmitSummary)
112131
if submitErr != nil {
113-
return formatReplyError(submitErr, opts, "failed to submit pending review")
132+
return formatReplyError(submitErr, "failed to submit pending review")
114133
}
115134
if submitted == 0 {
116135
return fmt.Errorf("no pending reviews owned by %s found on pull request #%d", reviewer, opts.Pull)
117136
}
118137

119-
reply, err = service.ReplyToComment(ctx, opts.Org, opts.Repo, opts.Pull, opts.CommentID, opts.Body)
138+
reply, err = service.ReplyToComment(ctx, owner, name, opts.Pull, opts.CommentID, opts.Body)
120139
if err != nil {
121-
return formatReplyError(err, opts, "failed to post reply after submitting pending review")
140+
return formatReplyError(err, "failed to post reply after submitting pending review")
122141
}
123142
} else {
124-
return formatReplyError(err, opts, "failed to post reply")
143+
return formatReplyError(err, "failed to post reply")
125144
}
126145
}
127146

@@ -134,7 +153,7 @@ func runReplyComment(ctx context.Context, opts *ReplyCommentOptions) error {
134153
return nil
135154
}
136155

137-
func formatReplyError(err error, opts *ReplyCommentOptions, prefix string) error {
156+
func formatReplyError(err error, prefix string) error {
138157
switch e := err.(type) {
139158
case *reviewapi.PullRequestNotFoundError:
140159
return fmt.Errorf("%s: %w", prefix, e)

pkg/cmd/pr/reply-comment/reply_comment_test.go

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package reply_comment
33
import (
44
"bytes"
55
"encoding/json"
6+
"errors"
67
"io"
78
"net/http"
89
"net/url"
@@ -11,6 +12,7 @@ import (
1112

1213
"github.com/cli/cli/v2/internal/config"
1314
"github.com/cli/cli/v2/internal/gh"
15+
"github.com/cli/cli/v2/internal/ghrepo"
1416
"github.com/cli/cli/v2/pkg/cmdutil"
1517
"github.com/cli/cli/v2/pkg/httpmock"
1618
"github.com/cli/cli/v2/pkg/iostreams"
@@ -19,7 +21,7 @@ import (
1921
"github.com/stretchr/testify/require"
2022
)
2123

22-
func newTestFactory(t *testing.T, rt http.RoundTripper) (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffer) {
24+
func newTestFactory(t *testing.T, rt http.RoundTripper, repo ghrepo.Interface, repoErr error) (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffer) {
2325
t.Helper()
2426
ios, _, stdout, stderr := iostreams.Test()
2527
ios.SetStdoutTTY(false)
@@ -29,15 +31,27 @@ func newTestFactory(t *testing.T, rt http.RoundTripper) (*cmdutil.Factory, *byte
2931
cfg := config.NewBlankConfig()
3032
cfg.Authentication().SetDefaultHost("github.com", "default")
3133

32-
return &cmdutil.Factory{
34+
factory := &cmdutil.Factory{
3335
IOStreams: ios,
3436
HttpClient: func() (*http.Client, error) {
3537
return &http.Client{Transport: rt}, nil
3638
},
3739
Config: func() (gh.Config, error) {
3840
return cfg, nil
3941
},
40-
}, stdout, stderr
42+
}
43+
44+
factory.BaseRepo = func() (ghrepo.Interface, error) {
45+
if repoErr != nil {
46+
return nil, repoErr
47+
}
48+
if repo != nil {
49+
return repo, nil
50+
}
51+
return ghrepo.FromFullName("ORG/REPO")
52+
}
53+
54+
return factory, stdout, stderr
4155
}
4256

4357
func TestReplyComment_AutoSubmitPending(t *testing.T) {
@@ -98,11 +112,13 @@ func TestReplyComment_AutoSubmitPending(t *testing.T) {
98112
httpmock.JSONResponse(reply),
99113
)
100114

101-
f, stdout, stderr := newTestFactory(t, reg)
115+
repo, err := ghrepo.FromFullName("ORG/REPO")
116+
require.NoError(t, err)
117+
f, stdout, stderr := newTestFactory(t, reg, repo, nil)
102118
cmd := NewCmdReplyComment(f)
103-
cmd.SetArgs([]string{"--org", "ORG", "--repo", "REPO", "--pr", "123", "--comment-id", "456", "--body", "Thanks!", "--auto-submit-pending"})
119+
cmd.SetArgs([]string{"123", "--comment-id", "456", "--body", "Thanks!", "--auto-submit-pending"})
104120

105-
_, err := cmd.ExecuteC()
121+
_, err = cmd.ExecuteC()
106122
require.NoError(t, err)
107123
assert.Equal(t, "", stderr.String())
108124
assert.JSONEq(t, `{"id":333,"in_reply_to_id":456,"body":"Thanks!","user":{"login":"octocat"},"created_at":"2024-05-06T07:08:09Z"}`, stdout.String())
@@ -115,3 +131,52 @@ func TestReplyComment_AutoSubmitPending(t *testing.T) {
115131
}
116132
assert.Equal(t, 2, reqCount, "expected two requests to reply endpoint")
117133
}
134+
135+
func TestReplyComment_RepoOverrideWithoutGit(t *testing.T) {
136+
reg := &httpmock.Registry{}
137+
defer reg.Verify(t)
138+
139+
reg.Register(
140+
httpmock.WithHost(httpmock.REST("POST", "repos/octo/demo/pulls/10/comments/50/replies"), "api.github.com"),
141+
httpmock.JSONResponse(map[string]interface{}{"id": 99}),
142+
)
143+
144+
noRepoErr := errors.New("no repository")
145+
f, stdout, stderr := newTestFactory(t, reg, nil, noRepoErr)
146+
f.BaseRepo = cmdutil.OverrideBaseRepoFunc(f, "octo/demo")
147+
148+
cmd := NewCmdReplyComment(f)
149+
cmd.SetArgs([]string{"10", "--comment-id", "50", "--body", "Thanks"})
150+
151+
_, err := cmd.ExecuteC()
152+
require.NoError(t, err)
153+
assert.Equal(t, "", stderr.String())
154+
155+
var payload map[string]interface{}
156+
require.NoError(t, json.Unmarshal(stdout.Bytes(), &payload))
157+
assert.Equal(t, float64(99), payload["id"])
158+
}
159+
160+
func TestReplyComment_FullReferenceWithoutGit(t *testing.T) {
161+
reg := &httpmock.Registry{}
162+
defer reg.Verify(t)
163+
164+
reg.Register(
165+
httpmock.WithHost(httpmock.REST("POST", "repos/octo/demo/pulls/10/comments/50/replies"), "api.github.com"),
166+
httpmock.JSONResponse(map[string]interface{}{"id": 101}),
167+
)
168+
169+
noRepoErr := errors.New("no repository")
170+
f, stdout, stderr := newTestFactory(t, reg, nil, noRepoErr)
171+
172+
cmd := NewCmdReplyComment(f)
173+
cmd.SetArgs([]string{"octo/demo#10", "--comment-id", "50", "--body", "ack"})
174+
175+
_, err := cmd.ExecuteC()
176+
require.NoError(t, err)
177+
assert.Equal(t, "", stderr.String())
178+
179+
var payload map[string]interface{}
180+
require.NoError(t, json.Unmarshal(stdout.Bytes(), &payload))
181+
assert.Equal(t, float64(101), payload["id"])
182+
}

0 commit comments

Comments
 (0)