Repository: google/go-github
Pull request #1484 (2020-04-08 21:38:28)

Update IdP team-sync list and create/update endpoints with ByID and BySlug
Addresses Issue #1483 
Changes include:
* Update method signatures of `#ListIDPGroupsForTeam` and `#CreateOrUpdateIDPGroupConnections`  to include the organization name identified as `org` and rename the `teamID` attribute to `slug` as it can be misleading/the api requires the slug for these particular calls
* Update respective tests with corrected arguments

googlebot: 
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: **Please visit <https://cla.developers.google.com/> to sign.**

Once you've signed (or fixed any issues), please reply here with `@googlebot I signed it!` and we'll verify it.

----

#### What to do if you already signed the CLA

##### Individual signers

*   It's possible we don't have your GitHub username or you're using a different email address on your commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).

##### Corporate signers

*   Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to [go/cla#troubleshoot](http://go/cla#troubleshoot) ([Public version](https://opensource.google/docs/cla/#troubleshoot)).
*   The email used to register you as an authorized contributor must be the email used for the Git commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).
*   The email used to register you as an authorized contributor must also be [attached to your GitHub account](https://github.com/settings/emails).
		

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fgo-github%2Fpull%2F1484) for more info**.

<!-- need_sender_cla -->
codecov[bot]: # [Codecov](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=h1) Report
> Merging [#1484](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-github/commit/4a7cf4a76843a3e11ce2bfd96a9b5067a07a2429&el=desc) will **increase** coverage by `5.09%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-github/pull/1484/graphs/tree.svg?width=650&height=150&src=pr&token=EgYjGjZlI0)](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1484      +/-   ##
==========================================
+ Coverage   68.16%   73.25%   +5.09%     
==========================================
  Files          94       94              
  Lines        8436     6794    -1642     
==========================================
- Hits         5750     4977     -773     
+ Misses       1823      954     -869     
  Partials      863      863              
```


| [Impacted Files](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [github/teams.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL3RlYW1zLmdv) | `73.41% <100.00%> (+5.68%)` | :arrow_up: |
| [github/github.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL2dpdGh1Yi5nbw==) | `89.62% <0.00%> (-0.03%)` | :arrow_down: |
| [github/admin.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL2FkbWluLmdv) | `100.00% <0.00%> (ø)` | |
| [github/strings.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL3N0cmluZ3MuZ28=) | `100.00% <0.00%> (ø)` | |
| [github/activity.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL2FjdGl2aXR5Lmdv) | `100.00% <0.00%> (ø)` | |
| [github/timestamp.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL3RpbWVzdGFtcC5nbw==) | `100.00% <0.00%> (ø)` | |
| [github/admin\_stats.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL2FkbWluX3N0YXRzLmdv) | `100.00% <0.00%> (ø)` | |
| [github/repos\_traffic.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL3JlcG9zX3RyYWZmaWMuZ28=) | `100.00% <0.00%> (ø)` | |
| [github/pulls\_reviewers.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL3B1bGxzX3Jldmlld2Vycy5nbw==) | `100.00% <0.00%> (ø)` | |
| [github/git\_commits.go](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree#diff-Z2l0aHViL2dpdF9jb21taXRzLmdv) | `91.30% <0.00%> (+1.30%)` | :arrow_up: |
| ... and [82 more](https://codecov.io/gh/google/go-github/pull/1484/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=footer). Last update [4a7cf4a...295a85f](https://codecov.io/gh/google/go-github/pull/1484?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

anGie44: @googlebot I signed it!
googlebot: CLAs look good, thanks!

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fgo-github%2Fpull%2F1484) for more info**.

<!-- ok -->
anGie44: Hi @gmlewis, thank you for the feedback!  I'm totally onboard with supporting both endpoint versions. I'll submit a change shortly :) 
gmlewis: Thank you, @wesleimp!
Merging.

Note to self: this is a breaking API change which means that the next release must be `v31.0.0`.

Note to others: we only make new releases now on-demand which means that we will continue to address issues and update the repo without releasing until someone says "Hey, we want to use the feature in that latest PR for this huge project and we would like to please have a new release" or something to that effect.  :smile:

Patch:
diff --git a/github/teams.go b/github/teams.go
index cfb814e7..9ca449cd 100644
--- a/github/teams.go
+++ b/github/teams.go
@@ -765,11 +765,12 @@ func (s *TeamsService) ListIDPGroupsInOrganization(ctx context.Context, org stri
 	return groups, resp, nil
 }
 
-// ListIDPGroupsForTeam lists IDP groups connected to a team on GitHub.
+// ListIDPGroupsForTeamByID lists IDP groups connected to a team on GitHub
+// given organization and team IDs.
 //
 // GitHub API docs: https://developer.github.com/v3/teams/team_sync/#list-idp-groups-for-a-team
-func (s *TeamsService) ListIDPGroupsForTeam(ctx context.Context, teamID string) (*IDPGroupList, *Response, error) {
-	u := fmt.Sprintf("teams/%v/team-sync/group-mappings", teamID)
+func (s *TeamsService) ListIDPGroupsForTeamByID(ctx context.Context, orgID, teamID int64) (*IDPGroupList, *Response, error) {
+	u := fmt.Sprintf("organizations/%v/team/%v/team-sync/group-mappings", orgID, teamID)
 
 	req, err := s.client.NewRequest("GET", u, nil)
 	if err != nil {
@@ -784,12 +785,53 @@ func (s *TeamsService) ListIDPGroupsForTeam(ctx context.Context, teamID string)
 	return groups, resp, err
 }
 
-// CreateOrUpdateIDPGroupConnections creates, updates, or removes a connection between a team
-// and an IDP group.
+// ListIDPGroupsForTeamBySlug lists IDP groups connected to a team on GitHub
+// given organization name and team slug.
+//
+// GitHub API docs: https://developer.github.com/v3/teams/team_sync/#list-idp-groups-for-a-team
+func (s *TeamsService) ListIDPGroupsForTeamBySlug(ctx context.Context, org, slug string) (*IDPGroupList, *Response, error) {
+	u := fmt.Sprintf("orgs/%v/teams/%v/team-sync/group-mappings", org, slug)
+
+	req, err := s.client.NewRequest("GET", u, nil)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	groups := new(IDPGroupList)
+	resp, err := s.client.Do(ctx, req, groups)
+	if err != nil {
+		return nil, resp, err
+	}
+	return groups, resp, err
+}
+
+// CreateOrUpdateIDPGroupConnectionsByID creates, updates, or removes a connection
+// between a team and an IDP group given organization and team IDs.
+//
+// GitHub API docs: https://developer.github.com/v3/teams/team_sync/#create-or-update-idp-group-connections
+func (s *TeamsService) CreateOrUpdateIDPGroupConnectionsByID(ctx context.Context, orgID, teamID int64, opts IDPGroupList) (*IDPGroupList, *Response, error) {
+	u := fmt.Sprintf("organizations/%v/team/%v/team-sync/group-mappings", orgID, teamID)
+
+	req, err := s.client.NewRequest("PATCH", u, opts)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	groups := new(IDPGroupList)
+	resp, err := s.client.Do(ctx, req, groups)
+	if err != nil {
+		return nil, resp, err
+	}
+
+	return groups, resp, nil
+}
+
+// CreateOrUpdateIDPGroupConnectionsBySlug creates, updates, or removes a connection
+// between a team and an IDP group given organization name and team slug.
 //
 // GitHub API docs: https://developer.github.com/v3/teams/team_sync/#create-or-update-idp-group-connections
-func (s *TeamsService) CreateOrUpdateIDPGroupConnections(ctx context.Context, teamID string, opts IDPGroupList) (*IDPGroupList, *Response, error) {
-	u := fmt.Sprintf("teams/%v/team-sync/group-mappings", teamID)
+func (s *TeamsService) CreateOrUpdateIDPGroupConnectionsBySlug(ctx context.Context, org, slug string, opts IDPGroupList) (*IDPGroupList, *Response, error) {
+	u := fmt.Sprintf("orgs/%v/teams/%v/team-sync/group-mappings", org, slug)
 
 	req, err := s.client.NewRequest("PATCH", u, opts)
 	if err != nil {


Test patch:
diff --git a/github/teams_test.go b/github/teams_test.go
index 414ad78e..304f93c3 100644
--- a/github/teams_test.go
+++ b/github/teams_test.go
@@ -946,18 +946,18 @@ func TestTeamsService_ListIDPGroupsInOrganization(t *testing.T) {
 	}
 }
 
-func TestTeamsService_ListIDPGroupsForTeam(t *testing.T) {
+func TestTeamsService_ListIDPGroupsForTeamByID(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
-	mux.HandleFunc("/teams/1/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/organizations/1/team/1/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `{"groups": [{"group_id": "1",  "group_name": "n", "group_description": "d"}]}`)
 	})
 
-	groups, _, err := client.Teams.ListIDPGroupsForTeam(context.Background(), "1")
+	groups, _, err := client.Teams.ListIDPGroupsForTeamByID(context.Background(), 1, 1)
 	if err != nil {
-		t.Errorf("Teams.ListIDPGroupsForTeam returned error: %v", err)
+		t.Errorf("Teams.ListIDPGroupsForTeamByID returned error: %v", err)
 	}
 
 	want := &IDPGroupList{
@@ -970,15 +970,81 @@ func TestTeamsService_ListIDPGroupsForTeam(t *testing.T) {
 		},
 	}
 	if !reflect.DeepEqual(groups, want) {
-		t.Errorf("Teams.ListIDPGroupsForTeam returned %+v. want %+v", groups, want)
+		t.Errorf("Teams.ListIDPGroupsForTeamByID returned %+v. want %+v", groups, want)
 	}
 }
 
-func TestTeamsService_CreateOrUpdateIDPGroupConnections(t *testing.T) {
+func TestTeamsService_ListIDPGroupsForTeamBySlug(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
-	mux.HandleFunc("/teams/1/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/orgs/o/teams/slug/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{"groups": [{"group_id": "1",  "group_name": "n", "group_description": "d"}]}`)
+	})
+
+	groups, _, err := client.Teams.ListIDPGroupsForTeamBySlug(context.Background(), "o", "slug")
+	if err != nil {
+		t.Errorf("Teams.ListIDPGroupsForTeamBySlug returned error: %v", err)
+	}
+
+	want := &IDPGroupList{
+		Groups: []*IDPGroup{
+			{
+				GroupID:          String("1"),
+				GroupName:        String("n"),
+				GroupDescription: String("d"),
+			},
+		},
+	}
+	if !reflect.DeepEqual(groups, want) {
+		t.Errorf("Teams.ListIDPGroupsForTeamBySlug returned %+v. want %+v", groups, want)
+	}
+}
+
+func TestTeamsService_CreateOrUpdateIDPGroupConnectionsByID(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	mux.HandleFunc("/organizations/1/team/1/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "PATCH")
+		fmt.Fprint(w, `{"groups": [{"group_id": "1",  "group_name": "n", "group_description": "d"}]}`)
+	})
+
+	input := IDPGroupList{
+		Groups: []*IDPGroup{
+			{
+				GroupID:          String("1"),
+				GroupName:        String("n"),
+				GroupDescription: String("d"),
+			},
+		},
+	}
+
+	groups, _, err := client.Teams.CreateOrUpdateIDPGroupConnectionsByID(context.Background(), 1, 1, input)
+	if err != nil {
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsByID returned error: %v", err)
+	}
+
+	want := &IDPGroupList{
+		Groups: []*IDPGroup{
+			{
+				GroupID:          String("1"),
+				GroupName:        String("n"),
+				GroupDescription: String("d"),
+			},
+		},
+	}
+	if !reflect.DeepEqual(groups, want) {
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsByID returned %+v. want %+v", groups, want)
+	}
+}
+
+func TestTeamsService_CreateOrUpdateIDPGroupConnectionsBySlug(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	mux.HandleFunc("/orgs/o/teams/slug/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "PATCH")
 		fmt.Fprint(w, `{"groups": [{"group_id": "1",  "group_name": "n", "group_description": "d"}]}`)
 	})
@@ -993,9 +1059,9 @@ func TestTeamsService_CreateOrUpdateIDPGroupConnections(t *testing.T) {
 		},
 	}
 
-	groups, _, err := client.Teams.CreateOrUpdateIDPGroupConnections(context.Background(), "1", input)
+	groups, _, err := client.Teams.CreateOrUpdateIDPGroupConnectionsBySlug(context.Background(), "o", "slug", input)
 	if err != nil {
-		t.Errorf("Teams.CreateOrUpdateIDPGroupConnections returned error: %v", err)
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsBySlug returned error: %v", err)
 	}
 
 	want := &IDPGroupList{
@@ -1008,15 +1074,40 @@ func TestTeamsService_CreateOrUpdateIDPGroupConnections(t *testing.T) {
 		},
 	}
 	if !reflect.DeepEqual(groups, want) {
-		t.Errorf("Teams.CreateOrUpdateIDPGroupConnections returned %+v. want %+v", groups, want)
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsBySlug returned %+v. want %+v", groups, want)
+	}
+}
+func TestTeamsService_CreateOrUpdateIDPGroupConnectionsByID_empty(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	mux.HandleFunc("/organizations/1/team/1/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "PATCH")
+		fmt.Fprint(w, `{"groups": []}`)
+	})
+
+	input := IDPGroupList{
+		Groups: []*IDPGroup{},
+	}
+
+	groups, _, err := client.Teams.CreateOrUpdateIDPGroupConnectionsByID(context.Background(), 1, 1, input)
+	if err != nil {
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsByID returned error: %v", err)
+	}
+
+	want := &IDPGroupList{
+		Groups: []*IDPGroup{},
+	}
+	if !reflect.DeepEqual(groups, want) {
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsByID returned %+v. want %+v", groups, want)
 	}
 }
 
-func TestTeamsService_CreateOrUpdateIDPGroupConnections_empty(t *testing.T) {
+func TestTeamsService_CreateOrUpdateIDPGroupConnectionsBySlug_empty(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
-	mux.HandleFunc("/teams/1/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/orgs/o/teams/slug/team-sync/group-mappings", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "PATCH")
 		fmt.Fprint(w, `{"groups": []}`)
 	})
@@ -1025,15 +1116,15 @@ func TestTeamsService_CreateOrUpdateIDPGroupConnections_empty(t *testing.T) {
 		Groups: []*IDPGroup{},
 	}
 
-	groups, _, err := client.Teams.CreateOrUpdateIDPGroupConnections(context.Background(), "1", input)
+	groups, _, err := client.Teams.CreateOrUpdateIDPGroupConnectionsBySlug(context.Background(), "o", "slug", input)
 	if err != nil {
-		t.Errorf("Teams.CreateOrUpdateIDPGroupConnections returned error: %v", err)
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsBySlug returned error: %v", err)
 	}
 
 	want := &IDPGroupList{
 		Groups: []*IDPGroup{},
 	}
 	if !reflect.DeepEqual(groups, want) {
-		t.Errorf("Teams.CreateOrUpdateIDPGroupConnections returned %+v. want %+v", groups, want)
+		t.Errorf("Teams.CreateOrUpdateIDPGroupConnectionsBySlug returned %+v. want %+v", groups, want)
 	}
 }