Go-github: BUG > IDPGroupList.Groups should not be omitempty

[kuwas]

Description

Current Behavior

• IDPGroupList.Groups is set to omitempty
  
  
  
  • https://github.com/google/go-github/blob/master/github/teams.go#L499
  
  
• When IDPGroupList.Groups is set to an empty slice, groups key is omitted and the request body is just an empty JSON {}
• This means that the IdP team sync delete operation does not work
  
  
  
  • GitHub API is expecting the groups key set with an empty array []
  
  
  • Specifying an empty groups array will remove all connections for a team.
  
  
  • https://developer.github.com/v3/teams/team_sync/#create-or-update-idp-group-connections
  
  

Expected Behavior

• IDPGroupList.Groups is not set to omitempty

Additional Details

Code

    groups := make([]*github.IDPGroup, 0)
    opt := github.IDPGroupList{Groups: groups}
    _, _, err := client.Teams.CreateOrUpdateIDPGroupConnections(ctx, teamId, opt)

Request

2020-01-14T14:40:08.054-0500 [DEBUG] plugin.terraform-provider-github: 2020/01/14 14:40:08 [DEBUG] Github API Request Details:
2020-01-14T14:40:08.054-0500 [DEBUG] plugin.terraform-provider-github: ---[ REQUEST ]---------------------------------------
2020-01-14T14:40:08.054-0500 [DEBUG] plugin.terraform-provider-github: PATCH /teams/#######/team-sync/group-mappings HTTP/1.1
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: Host: api.github.com
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: User-Agent: go-github
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: Content-Length: 3
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: Accept: application/vnd.github.team-sync-preview+json
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: Content-Type: application/json
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: Accept-Encoding: gzip
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github:
2020-01-14T14:40:08.055-0500 [DEBUG] plugin.terraform-provider-github: {}

[gmlewis]

Reading through the docs and the code, I don't see any downside to removing the ,omitempty on the Groups field.

Noting that the IDPGroupList struct is used both as a method parameter and is also returned from methods, it seems to me that since Groups is a slice, it should have no effect on existing code.

Thank you for reporting this, @kuwas. Would you like to put together a PR to remove that field tag modifier and update the unit tests if necessary to demonstrate the new behavior?

[kuwas]

@gmlewis I can give it a shot tomorrow