Skip to content

Commit d098964

Browse files
author
Nate Smith
authored
Merge pull request cli#3351 from cristiand391/fix-pr-reopen
Fix detecting PR status when passing branch as arg
2 parents 882c081 + a892116 commit d098964

File tree

7 files changed

+55
-14
lines changed

7 files changed

+55
-14
lines changed

‎api/queries_pr.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ type PullRequest struct {
3434
Number int
3535
Title string
3636
State string
37-
Closed bool
3837
URL string
3938
BaseRefName string
4039
HeadRefName string
@@ -165,6 +164,10 @@ func (pr PullRequest) Identifier() string {
165164
return pr.ID
166165
}
167166

167+
func (pr PullRequest) IsOpen() bool {
168+
return pr.State == "OPEN"
169+
}
170+
168171
type PullRequestReviewStatus struct {
169172
ChangesRequested bool
170173
Approved bool

‎pkg/cmd/pr/close/close.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func closeRun(opts *CloseOptions) error {
7676
if pr.State == "MERGED" {
7777
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be closed because it was already merged", cs.Red("!"), pr.Number, pr.Title)
7878
return cmdutil.SilentError
79-
} else if pr.Closed {
79+
} else if !pr.IsOpen() {
8080
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already closed\n", cs.Yellow("!"), pr.Number, pr.Title)
8181
return nil
8282
}

‎pkg/cmd/pr/close/close_test.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestPrClose(t *testing.T) {
6767
httpmock.GraphQL(`query PullRequestByNumber\b`),
6868
httpmock.StringResponse(`
6969
{ "data": { "repository": {
70-
"pullRequest": { "id": "THE-ID", "number": 96, "title": "The title of the PR" }
70+
"pullRequest": { "id": "THE-ID", "number": 96, "title": "The title of the PR", "state": "OPEN" }
7171
} } }`),
7272
)
7373
http.Register(
@@ -98,7 +98,7 @@ func TestPrClose_alreadyClosed(t *testing.T) {
9898
httpmock.GraphQL(`query PullRequestByNumber\b`),
9999
httpmock.StringResponse(`
100100
{ "data": { "repository": {
101-
"pullRequest": { "number": 101, "title": "The title of the PR", "closed": true }
101+
"pullRequest": { "number": 101, "title": "The title of the PR", "state": "CLOSED" }
102102
} } }`),
103103
)
104104

@@ -121,8 +121,13 @@ func TestPrClose_deleteBranch(t *testing.T) {
121121
http.Register(
122122
httpmock.GraphQL(`query PullRequestByNumber\b`),
123123
httpmock.StringResponse(`
124-
{ "data": { "repository": {
125-
"pullRequest": { "id": "THE-ID", "number": 96, "title": "The title of the PR", "headRefName":"blueberries", "headRepositoryOwner": {"login": "OWNER"}}
124+
{ "data": { "repository": { "pullRequest": {
125+
"id": "THE-ID",
126+
"number": 96,
127+
"title": "The title of the PR",
128+
"headRefName":"blueberries",
129+
"headRepositoryOwner": {"login": "OWNER"},
130+
"state": "OPEN" }
126131
} } }`),
127132
)
128133
http.Register(

‎pkg/cmd/pr/ready/ready.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func readyRun(opts *ReadyOptions) error {
8282
return err
8383
}
8484

85-
if pr.Closed {
85+
if !pr.IsOpen() {
8686
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", cs.Red("!"), pr.Number)
8787
return cmdutil.SilentError
8888
} else if !pr.IsDraft {

‎pkg/cmd/pr/ready/ready_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func TestPRReady(t *testing.T) {
147147
httpmock.GraphQL(`query PullRequestByNumber\b`),
148148
httpmock.StringResponse(`
149149
{ "data": { "repository": {
150-
"pullRequest": { "id": "THE-ID", "number": 444, "closed": false, "isDraft": true}
150+
"pullRequest": { "id": "THE-ID", "number": 444, "state": "OPEN", "isDraft": true}
151151
} } }`),
152152
)
153153
http.Register(
@@ -178,7 +178,7 @@ func TestPRReady_alreadyReady(t *testing.T) {
178178
httpmock.GraphQL(`query PullRequestByNumber\b`),
179179
httpmock.StringResponse(`
180180
{ "data": { "repository": {
181-
"pullRequest": { "number": 445, "closed": false, "isDraft": false}
181+
"pullRequest": { "number": 445, "state": "OPEN", "isDraft": false}
182182
} } }`),
183183
)
184184

@@ -202,7 +202,7 @@ func TestPRReady_closed(t *testing.T) {
202202
httpmock.GraphQL(`query PullRequestByNumber\b`),
203203
httpmock.StringResponse(`
204204
{ "data": { "repository": {
205-
"pullRequest": { "number": 446, "closed": true, "isDraft": true}
205+
"pullRequest": { "number": 446, "state": "CLOSED", "isDraft": true}
206206
} } }`),
207207
)
208208

‎pkg/cmd/pr/reopen/reopen.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func reopenRun(opts *ReopenOptions) error {
7070
return cmdutil.SilentError
7171
}
7272

73-
if !pr.Closed {
73+
if pr.IsOpen() {
7474
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already open\n", cs.Yellow("!"), pr.Number, pr.Title)
7575
return nil
7676
}

‎pkg/cmd/pr/reopen/reopen_test.go‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestPRReopen(t *testing.T) {
6363
httpmock.GraphQL(`query PullRequestByNumber\b`),
6464
httpmock.StringResponse(`
6565
{ "data": { "repository": {
66-
"pullRequest": { "id": "THE-ID", "number": 666, "title": "The title of the PR", "closed": true}
66+
"pullRequest": { "id": "THE-ID", "number": 666, "title": "The title of the PR", "state": "CLOSED" }
6767
} } }`),
6868
)
6969
http.Register(
@@ -86,6 +86,39 @@ func TestPRReopen(t *testing.T) {
8686
}
8787
}
8888

89+
func TestPRReopen_BranchArg(t *testing.T) {
90+
http := &httpmock.Registry{}
91+
defer http.Verify(t)
92+
93+
http.Register(
94+
httpmock.GraphQL(`query PullRequestForBranch\b`),
95+
httpmock.StringResponse(`
96+
{ "data": { "repository": { "pullRequests": {
97+
"nodes": [
98+
{ "id": "THE-ID", "number": 666, "title": "The title of the PR", "headRefName": "fix-bug", "state": "CLOSED" }
99+
]
100+
} } } }`),
101+
)
102+
http.Register(
103+
httpmock.GraphQL(`mutation PullRequestReopen\b`),
104+
httpmock.GraphQLMutation(`{"id": "THE-ID"}`,
105+
func(inputs map[string]interface{}) {
106+
assert.Equal(t, inputs["pullRequestId"], "THE-ID")
107+
}),
108+
)
109+
110+
output, err := runCommand(http, true, "fix-bug")
111+
if err != nil {
112+
t.Fatalf("error running command `pr reopen`: %v", err)
113+
}
114+
115+
r := regexp.MustCompile(`Reopened pull request #666 \(The title of the PR\)`)
116+
117+
if !r.MatchString(output.Stderr()) {
118+
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
119+
}
120+
}
121+
89122
func TestPRReopen_alreadyOpen(t *testing.T) {
90123
http := &httpmock.Registry{}
91124
defer http.Verify(t)
@@ -94,7 +127,7 @@ func TestPRReopen_alreadyOpen(t *testing.T) {
94127
httpmock.GraphQL(`query PullRequestByNumber\b`),
95128
httpmock.StringResponse(`
96129
{ "data": { "repository": {
97-
"pullRequest": { "number": 666, "title": "The title of the PR", "closed": false}
130+
"pullRequest": { "number": 666, "title": "The title of the PR", "state": "OPEN" }
98131
} } }`),
99132
)
100133

@@ -118,7 +151,7 @@ func TestPRReopen_alreadyMerged(t *testing.T) {
118151
httpmock.GraphQL(`query PullRequestByNumber\b`),
119152
httpmock.StringResponse(`
120153
{ "data": { "repository": {
121-
"pullRequest": { "number": 666, "title": "The title of the PR", "closed": true, "state": "MERGED"}
154+
"pullRequest": { "number": 666, "title": "The title of the PR", "state": "MERGED"}
122155
} } }`),
123156
)
124157

0 commit comments

Comments
 (0)