Skip to content

Commit 3159217

Browse files
authored
Use graphql rather than rest API for Github team membership (#1786)
This change should provide better diffing for team membership for teams with child teams. Currently, child team members are included in the parent team's members list and so diffs are incorrect. See [Issue 1193](#1193).
1 parent b490535 commit 3159217

File tree

2 files changed

+27
-66
lines changed

2 files changed

+27
-66
lines changed

github/resource_github_team_members.go

Lines changed: 27 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"context"
55
"log"
66
"reflect"
7+
"strings"
78

89
"github.com/google/go-github/v53/github"
910
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
11+
"github.com/shurcooL/githubv4"
1012
)
1113

1214
type MemberChange struct {
@@ -53,10 +55,6 @@ func resourceGithubTeamMembers() *schema.Resource {
5355
},
5456
},
5557
},
56-
"etag": {
57-
Type: schema.TypeString,
58-
Computed: true,
59-
},
6058
},
6159
}
6260
}
@@ -176,15 +174,15 @@ func resourceGithubTeamMembersUpdate(d *schema.ResourceData, meta interface{}) e
176174
}
177175

178176
func resourceGithubTeamMembersRead(d *schema.ResourceData, meta interface{}) error {
179-
client := meta.(*Owner).v3client
180-
orgId := meta.(*Owner).id
177+
client := meta.(*Owner).v4client
178+
orgName := meta.(*Owner).name
181179
teamIdString := d.Get("team_id").(string)
182180
if teamIdString == "" && !d.IsNewResource() {
183181
log.Printf("[DEBUG] Importing team with id %q", d.Id())
184182
teamIdString = d.Id()
185183
}
186184

187-
teamId, err := getTeamID(teamIdString, meta)
185+
teamSlug, err := getTeamSlug(teamIdString, meta)
188186
if err != nil {
189187
return err
190188
}
@@ -195,81 +193,45 @@ func resourceGithubTeamMembersRead(d *schema.ResourceData, meta interface{}) err
195193
d.Set("team_id", teamIdString)
196194

197195
ctx := context.WithValue(context.Background(), ctxId, d.Id())
198-
if !d.IsNewResource() {
199-
ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string))
200-
}
201196

202-
etags := make([]string, 0)
203-
// List members & maintainers as list 'all' drops role information
204197
log.Printf("[DEBUG] Reading team members: %s", teamIdString)
205-
memberOptions := github.TeamListTeamMembersOptions{
206-
ListOptions: github.ListOptions{
207-
PerPage: maxPerPage,
208-
},
209-
Role: "member",
198+
var q struct {
199+
Organization struct {
200+
Team struct {
201+
Members struct {
202+
Edges []struct {
203+
Node struct {
204+
Login string
205+
}
206+
Role string
207+
}
208+
} `graphql:"members(membership:IMMEDIATE)"`
209+
} `graphql:"team(slug:$teamSlug)"`
210+
} `graphql:"organization(login:$orgName)"`
210211
}
211212

212-
var members []*github.User
213-
for {
214-
member, resp, err := client.Teams.ListTeamMembersByID(ctx, orgId, teamId, &memberOptions)
215-
if err != nil {
216-
return err
217-
}
218-
219-
etags = append(etags, resp.Header.Get("ETag"))
220-
members = append(members, member...)
221-
if resp.NextPage == 0 {
222-
break
223-
}
224-
memberOptions.Page = resp.NextPage
213+
variables := map[string]interface{}{
214+
"teamSlug": githubv4.String(teamSlug),
215+
"orgName": githubv4.String(orgName),
225216
}
226217

227-
log.Printf("[DEBUG] Reading team maintainers: %s", teamIdString)
228-
maintainerOptions := github.TeamListTeamMembersOptions{
229-
ListOptions: github.ListOptions{
230-
PerPage: maxPerPage,
231-
},
232-
Role: "maintainer",
233-
}
234-
var maintainers []*github.User
235-
for {
236-
maintaner, resp, err := client.Teams.ListTeamMembersByID(ctx, orgId, teamId, &maintainerOptions)
237-
if err != nil {
238-
return err
239-
}
240-
241-
etags = append(etags, resp.Header.Get("ETag"))
242-
maintainers = append(maintainers, maintaner...)
243-
244-
if resp.NextPage == 0 {
245-
break
246-
}
247-
maintainerOptions.Page = resp.NextPage
218+
if err := client.Query(ctx, &q, variables); err != nil {
219+
return err
248220
}
249221

250-
teamMembersAndMaintainers := make([]interface{}, len(members)+len(maintainers))
222+
teamMembersAndMaintainers := make([]interface{}, len(q.Organization.Team.Members.Edges))
251223
// Add all members to the list
252-
for i, member := range members {
224+
for i, member := range q.Organization.Team.Members.Edges {
253225
teamMembersAndMaintainers[i] = map[string]interface{}{
254-
"username": member.Login,
255-
"role": "member",
256-
}
257-
}
258-
// Add all maintainers to the list
259-
for i, member := range maintainers {
260-
teamMembersAndMaintainers[i+len(members)] = map[string]interface{}{
261-
"username": member.Login,
262-
"role": "maintainer",
226+
"username": member.Node.Login,
227+
"role": strings.ToLower(member.Role),
263228
}
264229
}
265230

266231
if err := d.Set("members", teamMembersAndMaintainers); err != nil {
267232
return err
268233
}
269234

270-
// Combine etag of all requests
271-
d.Set("etag", buildChecksumID(etags))
272-
273235
return nil
274236
}
275237

github/resource_github_team_members_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ func TestAccGithubTeamMembers(t *testing.T) {
3232
{
3333
Config: testAccGithubTeamMembersConfig(randomID, testCollaborator, "member"),
3434
Check: resource.ComposeTestCheckFunc(
35-
resource.TestCheckResourceAttrSet(resourceName, "etag"),
3635
testAccCheckGithubTeamMembersExists(resourceName, &membership),
3736
testAccCheckGithubTeamMembersRoleState(resourceName, "member", &membership),
3837
),

0 commit comments

Comments
 (0)