Skip to content

Commit 02f4946

Browse files
authored
Merge pull request cli#2190 from quiye/issues-data-race
Fix issue label overrides caused by pagination
2 parents 2565c9d + e270cdb commit 02f4946

File tree

5 files changed

+94
-9
lines changed

5 files changed

+94
-9
lines changed

api/queries_issue.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
270270
variables["milestone"] = milestoneRESTID
271271
}
272272

273-
var response struct {
273+
type responseData struct {
274274
Repository struct {
275275
Issues struct {
276276
TotalCount int
@@ -285,10 +285,12 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
285285
}
286286

287287
var issues []Issue
288+
var totalCount int
288289
pageLimit := min(limit, 100)
289290

290291
loop:
291292
for {
293+
var response responseData
292294
variables["limit"] = pageLimit
293295
err := client.GraphQL(repo.RepoHost(), query, variables, &response)
294296
if err != nil {
@@ -297,6 +299,7 @@ loop:
297299
if !response.Repository.HasIssuesEnabled {
298300
return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo))
299301
}
302+
totalCount = response.Repository.Issues.TotalCount
300303

301304
for _, issue := range response.Repository.Issues.Nodes {
302305
issues = append(issues, issue)
@@ -313,7 +316,7 @@ loop:
313316
}
314317
}
315318

316-
res := IssuesAndTotalCount{Issues: issues, TotalCount: response.Repository.Issues.TotalCount}
319+
res := IssuesAndTotalCount{Issues: issues, TotalCount: totalCount}
317320
return &res, nil
318321
}
319322

api/queries_issue_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"io/ioutil"
77
"testing"
88

9+
"github.com/stretchr/testify/assert"
10+
911
"github.com/cli/cli/internal/ghrepo"
1012
"github.com/cli/cli/pkg/httpmock"
1113
)
@@ -68,3 +70,76 @@ func TestIssueList(t *testing.T) {
6870
t.Errorf("expected %q, got %q", "ENDCURSOR", endCursor)
6971
}
7072
}
73+
74+
func TestIssueList_pagination(t *testing.T) {
75+
http := &httpmock.Registry{}
76+
client := NewClient(ReplaceTripper(http))
77+
78+
http.StubResponse(200, bytes.NewBufferString(`
79+
{ "data": { "repository": {
80+
"hasIssuesEnabled": true,
81+
"issues": {
82+
"nodes": [
83+
{
84+
"title": "issue1",
85+
"labels": { "nodes": [ { "name": "bug" } ], "totalCount": 1 },
86+
"assignees": { "nodes": [ { "login": "user1" } ], "totalCount": 1 }
87+
}
88+
],
89+
"pageInfo": {
90+
"hasNextPage": true,
91+
"endCursor": "ENDCURSOR"
92+
},
93+
"totalCount": 2
94+
}
95+
} } }
96+
`))
97+
http.StubResponse(200, bytes.NewBufferString(`
98+
{ "data": { "repository": {
99+
"hasIssuesEnabled": true,
100+
"issues": {
101+
"nodes": [
102+
{
103+
"title": "issue2",
104+
"labels": { "nodes": [ { "name": "enhancement" } ], "totalCount": 1 },
105+
"assignees": { "nodes": [ { "login": "user2" } ], "totalCount": 1 }
106+
}
107+
],
108+
"pageInfo": {
109+
"hasNextPage": false,
110+
"endCursor": "ENDCURSOR"
111+
},
112+
"totalCount": 2
113+
}
114+
} } }
115+
`))
116+
117+
repo := ghrepo.New("OWNER", "REPO")
118+
res, err := IssueList(client, repo, "", nil, "", 0, "", "", "")
119+
if err != nil {
120+
t.Fatalf("IssueList() error = %v", err)
121+
}
122+
123+
assert.Equal(t, 2, res.TotalCount)
124+
assert.Equal(t, 2, len(res.Issues))
125+
126+
getLabels := func(i Issue) []string {
127+
var labels []string
128+
for _, l := range i.Labels.Nodes {
129+
labels = append(labels, l.Name)
130+
}
131+
return labels
132+
}
133+
getAssignees := func(i Issue) []string {
134+
var logins []string
135+
for _, u := range i.Assignees.Nodes {
136+
logins = append(logins, u.Login)
137+
}
138+
return logins
139+
}
140+
141+
assert.Equal(t, []string{"bug"}, getLabels(res.Issues[0]))
142+
assert.Equal(t, []string{"user1"}, getAssignees(res.Issues[0]))
143+
assert.Equal(t, []string{"enhancement"}, getLabels(res.Issues[1]))
144+
assert.Equal(t, []string{"user2"}, getAssignees(res.Issues[1]))
145+
}

api/queries_org.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99

1010
// OrganizationProjects fetches all open projects for an organization
1111
func OrganizationProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) {
12-
var query struct {
12+
type responseData struct {
1313
Organization struct {
1414
Projects struct {
1515
Nodes []RepoProject
@@ -30,6 +30,7 @@ func OrganizationProjects(client *Client, repo ghrepo.Interface) ([]RepoProject,
3030

3131
var projects []RepoProject
3232
for {
33+
var query responseData
3334
err := gql.QueryNamed(context.Background(), "OrganizationProjectList", &query, variables)
3435
if err != nil {
3536
return nil, err
@@ -52,7 +53,7 @@ type OrgTeam struct {
5253

5354
// OrganizationTeams fetches all the teams in an organization
5455
func OrganizationTeams(client *Client, repo ghrepo.Interface) ([]OrgTeam, error) {
55-
var query struct {
56+
type responseData struct {
5657
Organization struct {
5758
Teams struct {
5859
Nodes []OrgTeam
@@ -73,6 +74,7 @@ func OrganizationTeams(client *Client, repo ghrepo.Interface) ([]OrgTeam, error)
7374

7475
var teams []OrgTeam
7576
for {
77+
var query responseData
7678
err := gql.QueryNamed(context.Background(), "OrganizationTeamList", &query, variables)
7779
if err != nil {
7880
return nil, err

api/queries_repo.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ type RepoProject struct {
657657

658658
// RepoProjects fetches all open projects for a repository
659659
func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) {
660-
var query struct {
660+
type responseData struct {
661661
Repository struct {
662662
Projects struct {
663663
Nodes []RepoProject
@@ -679,6 +679,7 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error)
679679

680680
var projects []RepoProject
681681
for {
682+
var query responseData
682683
err := gql.QueryNamed(context.Background(), "RepositoryProjectList", &query, variables)
683684
if err != nil {
684685
return nil, err
@@ -701,7 +702,7 @@ type RepoAssignee struct {
701702

702703
// RepoAssignableUsers fetches all the assignable users for a repository
703704
func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, error) {
704-
var query struct {
705+
type responseData struct {
705706
Repository struct {
706707
AssignableUsers struct {
707708
Nodes []RepoAssignee
@@ -723,6 +724,7 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee,
723724

724725
var users []RepoAssignee
725726
for {
727+
var query responseData
726728
err := gql.QueryNamed(context.Background(), "RepositoryAssignableUsers", &query, variables)
727729
if err != nil {
728730
return nil, err
@@ -745,7 +747,7 @@ type RepoLabel struct {
745747

746748
// RepoLabels fetches all the labels in a repository
747749
func RepoLabels(client *Client, repo ghrepo.Interface) ([]RepoLabel, error) {
748-
var query struct {
750+
type responseData struct {
749751
Repository struct {
750752
Labels struct {
751753
Nodes []RepoLabel
@@ -767,6 +769,7 @@ func RepoLabels(client *Client, repo ghrepo.Interface) ([]RepoLabel, error) {
767769

768770
var labels []RepoLabel
769771
for {
772+
var query responseData
770773
err := gql.QueryNamed(context.Background(), "RepositoryLabelList", &query, variables)
771774
if err != nil {
772775
return nil, err
@@ -789,7 +792,7 @@ type RepoMilestone struct {
789792

790793
// RepoMilestones fetches all open milestones in a repository
791794
func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, error) {
792-
var query struct {
795+
type responseData struct {
793796
Repository struct {
794797
Milestones struct {
795798
Nodes []RepoMilestone
@@ -811,6 +814,7 @@ func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, err
811814

812815
var milestones []RepoMilestone
813816
for {
817+
var query responseData
814818
err := gql.QueryNamed(context.Background(), "RepositoryMilestoneList", &query, variables)
815819
if err != nil {
816820
return nil, err

pkg/cmd/release/list/http.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type Release struct {
2121
}
2222

2323
func fetchReleases(httpClient *http.Client, repo ghrepo.Interface, limit int) ([]Release, error) {
24-
var query struct {
24+
type responseData struct {
2525
Repository struct {
2626
Releases struct {
2727
Nodes []Release
@@ -50,6 +50,7 @@ func fetchReleases(httpClient *http.Client, repo ghrepo.Interface, limit int) ([
5050
var releases []Release
5151
loop:
5252
for {
53+
var query responseData
5354
err := gql.QueryNamed(context.Background(), "RepositoryReleaseList", &query, variables)
5455
if err != nil {
5556
return nil, err

0 commit comments

Comments
 (0)