Skip to content

Commit 915cfbf

Browse files
nithish-95nithish-95
committed
fix: address code review feadback
- Use EnterpriseService instead of creating new service - Add EnterpriseInstallationRepositoriesOptions for consistency - Update method signatures to Required
1 parent d997ae6 commit 915cfbf

File tree

3 files changed

+33
-41
lines changed

3 files changed

+33
-41
lines changed

‎github/enterprise_apps.go‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,15 @@ import (
1010
"fmt"
1111
)
1212

13-
// EnterpriseAppsService handles communication with the enterprise apps related
14-
// methods of the GitHub API.
15-
//
16-
// GitHub API docs: https://docs.github.com/en/rest/reference/enterprise-admin#apps
17-
type EnterpriseAppsService service
18-
1913
// EnterpriseInstallationRepositoriesOptions specifies the parameters for
20-
// EnterpriseAppsService.AddRepositoriesToInstallation and
21-
// EnterpriseAppsService.RemoveRepositoriesFromInstallation.
14+
// EnterpriseService.AddRepositoriesToInstallation and
15+
// EnterpriseService.RemoveRepositoriesFromInstallation.
2216
type EnterpriseInstallationRepositoriesOptions struct {
2317
SelectedRepositoryIDs []int64 `json:"selected_repository_ids"`
2418
}
2519

2620
// EnterpriseInstallationRepositoriesToggleOptions specifies the parameters for
27-
// EnterpriseAppsService.ToggleInstallationRepositories.
21+
// EnterpriseService.ToggleInstallationRepositories.
2822
type EnterpriseInstallationRepositoriesToggleOptions struct {
2923
RepositorySelection *string `json:"repository_selection,omitempty"` // Can be "all" or "selected"
3024
SelectedRepositoryIDs []int64 `json:"selected_repository_ids,omitempty"`
@@ -36,7 +30,7 @@ type EnterpriseInstallationRepositoriesToggleOptions struct {
3630
// GitHub API docs: https://docs.github.com/enterprise-cloud@latest/rest/enterprise-admin/organization-installations#get-the-repositories-accessible-to-a-given-github-app-installation
3731
//
3832
//meta:operation GET /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories
39-
func (s *EnterpriseAppsService) ListRepositoriesForOrgInstallation(ctx context.Context, enterprise, org string, installationID int64, opts *ListOptions) (*ListRepositories, *Response, error) {
33+
func (s *EnterpriseService) ListRepositoriesForOrgInstallation(ctx context.Context, enterprise, org string, installationID int64, opts *ListOptions) (*ListRepositories, *Response, error) {
4034
u := fmt.Sprintf("enterprises/%v/apps/organizations/%v/installations/%v/repositories", enterprise, org, installationID)
4135
u, err := addOptions(u, opts)
4236
if err != nil {
@@ -63,7 +57,7 @@ func (s *EnterpriseAppsService) ListRepositoriesForOrgInstallation(ctx context.C
6357
// GitHub API docs: https://docs.github.com/enterprise-cloud@latest/rest/enterprise-admin/organization-installations#toggle-installation-repository-access-between-selected-and-all-repositories
6458
//
6559
//meta:operation PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories
66-
func (s *EnterpriseAppsService) ToggleInstallationRepositories(ctx context.Context, enterprise, org string, installationID int64, opts *EnterpriseInstallationRepositoriesToggleOptions) (*ListRepositories, *Response, error) {
60+
func (s *EnterpriseService) ToggleInstallationRepositories(ctx context.Context, enterprise, org string, installationID int64, opts *EnterpriseInstallationRepositoriesToggleOptions) (*ListRepositories, *Response, error) {
6761
u := fmt.Sprintf("enterprises/%v/apps/organizations/%v/installations/%v/repositories", enterprise, org, installationID)
6862
req, err := s.client.NewRequest("PATCH", u, opts)
6963
if err != nil {
@@ -84,7 +78,7 @@ func (s *EnterpriseAppsService) ToggleInstallationRepositories(ctx context.Conte
8478
// GitHub API docs: https://docs.github.com/enterprise-cloud@latest/rest/enterprise-admin/organization-installations#grant-repository-access-to-an-organization-installation
8579
//
8680
//meta:operation PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add
87-
func (s *EnterpriseAppsService) AddRepositoriesToInstallation(ctx context.Context, enterprise, org string, installationID int64, opts *EnterpriseInstallationRepositoriesOptions) (*ListRepositories, *Response, error) {
81+
func (s *EnterpriseService) AddRepositoriesToInstallation(ctx context.Context, enterprise, org string, installationID int64, opts EnterpriseInstallationRepositoriesOptions) (*ListRepositories, *Response, error) {
8882
u := fmt.Sprintf("enterprises/%v/apps/organizations/%v/installations/%v/repositories/add", enterprise, org, installationID)
8983
req, err := s.client.NewRequest("PATCH", u, opts)
9084
if err != nil {
@@ -105,7 +99,7 @@ func (s *EnterpriseAppsService) AddRepositoriesToInstallation(ctx context.Contex
10599
// GitHub API docs: https://docs.github.com/enterprise-cloud@latest/rest/enterprise-admin/organization-installations#remove-repository-access-from-an-organization-installation
106100
//
107101
//meta:operation PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/remove
108-
func (s *EnterpriseAppsService) RemoveRepositoriesFromInstallation(ctx context.Context, enterprise, org string, installationID int64, opts *EnterpriseInstallationRepositoriesOptions) (*ListRepositories, *Response, error) {
102+
func (s *EnterpriseService) RemoveRepositoriesFromInstallation(ctx context.Context, enterprise, org string, installationID int64, opts EnterpriseInstallationRepositoriesOptions) (*ListRepositories, *Response, error) {
109103
u := fmt.Sprintf("enterprises/%v/apps/organizations/%v/installations/%v/repositories/remove", enterprise, org, installationID)
110104
req, err := s.client.NewRequest("PATCH", u, opts)
111105
if err != nil {

‎github/enterprise_apps_test.go‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"github.com/google/go-cmp/cmp"
1414
)
1515

16-
func TestEnterpriseAppsService_ListRepositoriesForOrgInstallation(t *testing.T) {
16+
func TestEnterpriseService_ListRepositoriesForOrgInstallation(t *testing.T) {
1717
t.Parallel()
1818
client, mux, _ := setup(t)
1919

@@ -24,29 +24,29 @@ func TestEnterpriseAppsService_ListRepositoriesForOrgInstallation(t *testing.T)
2424
})
2525

2626
ctx := t.Context()
27-
repos, _, err := client.EnterpriseApps.ListRepositoriesForOrgInstallation(ctx, "e", "o", 1, &ListOptions{Page: 1})
27+
repos, _, err := client.Enterprise.ListRepositoriesForOrgInstallation(ctx, "e", "o", 1, &ListOptions{Page: 1})
2828
if err != nil {
29-
t.Errorf("EnterpriseApps.ListRepositoriesForOrgInstallation returned error: %v", err)
29+
t.Errorf("Enterprise.ListRepositoriesForOrgInstallation returned error: %v", err)
3030
}
3131

3232
want := &ListRepositories{TotalCount: Ptr(1), Repositories: []*Repository{{ID: Ptr(int64(1))}}}
3333
if diff := cmp.Diff(repos, want); diff != "" {
34-
t.Errorf("EnterpriseApps.ListRepositoriesForOrgInstallation returned diff (-want +got):\n%v", diff)
34+
t.Errorf("Enterprise.ListRepositoriesForOrgInstallation returned diff (-want +got):\n%v", diff)
3535
}
3636

3737
const methodName = "ListRepositoriesForOrgInstallation"
3838
testBadOptions(t, methodName, func() (err error) {
39-
_, _, err = client.EnterpriseApps.ListRepositoriesForOrgInstallation(ctx, "\n", "\n", -1, &ListOptions{})
39+
_, _, err = client.Enterprise.ListRepositoriesForOrgInstallation(ctx, "\n", "\n", -1, &ListOptions{})
4040
return err
4141
})
4242

4343
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
44-
_, resp, err := client.EnterpriseApps.ListRepositoriesForOrgInstallation(ctx, "e", "o", 1, &ListOptions{})
44+
_, resp, err := client.Enterprise.ListRepositoriesForOrgInstallation(ctx, "e", "o", 1, &ListOptions{})
4545
return resp, err
4646
})
4747
}
4848

49-
func TestEnterpriseAppsService_ToggleInstallationRepositories(t *testing.T) {
49+
func TestEnterpriseService_ToggleInstallationRepositories(t *testing.T) {
5050
t.Parallel()
5151
client, mux, _ := setup(t)
5252

@@ -62,33 +62,33 @@ func TestEnterpriseAppsService_ToggleInstallationRepositories(t *testing.T) {
6262
})
6363

6464
ctx := t.Context()
65-
repos, _, err := client.EnterpriseApps.ToggleInstallationRepositories(ctx, "e", "o", 1, input)
65+
repos, _, err := client.Enterprise.ToggleInstallationRepositories(ctx, "e", "o", 1, input)
6666
if err != nil {
67-
t.Errorf("EnterpriseApps.ToggleInstallationRepositories returned error: %v", err)
67+
t.Errorf("Enterprise.ToggleInstallationRepositories returned error: %v", err)
6868
}
6969

7070
want := &ListRepositories{TotalCount: Ptr(2), Repositories: []*Repository{{ID: Ptr(int64(1))}, {ID: Ptr(int64(2))}}}
7171
if diff := cmp.Diff(repos, want); diff != "" {
72-
t.Errorf("EnterpriseApps.ToggleInstallationRepositories returned diff (-want +got):\n%v", diff)
72+
t.Errorf("Enterprise.ToggleInstallationRepositories returned diff (-want +got):\n%v", diff)
7373
}
7474

7575
const methodName = "ToggleInstallationRepositories"
7676
testBadOptions(t, methodName, func() (err error) {
77-
_, _, err = client.EnterpriseApps.ToggleInstallationRepositories(ctx, "\n", "\n", -1, input)
77+
_, _, err = client.Enterprise.ToggleInstallationRepositories(ctx, "\n", "\n", -1, input)
7878
return err
7979
})
8080

8181
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
82-
_, resp, err := client.EnterpriseApps.ToggleInstallationRepositories(ctx, "e", "o", 1, input)
82+
_, resp, err := client.Enterprise.ToggleInstallationRepositories(ctx, "e", "o", 1, input)
8383
return resp, err
8484
})
8585
}
8686

87-
func TestEnterpriseAppsService_AddRepositoriesToInstallation(t *testing.T) {
87+
func TestEnterpriseService_AddRepositoriesToInstallation(t *testing.T) {
8888
t.Parallel()
8989
client, mux, _ := setup(t)
9090

91-
input := &EnterpriseInstallationRepositoriesOptions{SelectedRepositoryIDs: []int64{1, 2}}
91+
input := EnterpriseInstallationRepositoriesOptions{SelectedRepositoryIDs: []int64{1, 2}}
9292

9393
mux.HandleFunc("/enterprises/e/apps/organizations/o/installations/1/repositories/add", func(w http.ResponseWriter, r *http.Request) {
9494
testMethod(t, r, "PATCH")
@@ -97,33 +97,33 @@ func TestEnterpriseAppsService_AddRepositoriesToInstallation(t *testing.T) {
9797
})
9898

9999
ctx := t.Context()
100-
repos, _, err := client.EnterpriseApps.AddRepositoriesToInstallation(ctx, "e", "o", 1, input)
100+
repos, _, err := client.Enterprise.AddRepositoriesToInstallation(ctx, "e", "o", 1, input)
101101
if err != nil {
102-
t.Errorf("EnterpriseApps.AddRepositoriesToInstallation returned error: %v", err)
102+
t.Errorf("Enterprise.AddRepositoriesToInstallation returned error: %v", err)
103103
}
104104

105105
want := &ListRepositories{TotalCount: Ptr(2), Repositories: []*Repository{{ID: Ptr(int64(1))}, {ID: Ptr(int64(2))}}}
106106
if diff := cmp.Diff(repos, want); diff != "" {
107-
t.Errorf("EnterpriseApps.AddRepositoriesToInstallation returned diff (-want +got):\n%v", diff)
107+
t.Errorf("Enterprise.AddRepositoriesToInstallation returned diff (-want +got):\n%v", diff)
108108
}
109109

110110
const methodName = "AddRepositoriesToInstallation"
111111
testBadOptions(t, methodName, func() (err error) {
112-
_, _, err = client.EnterpriseApps.AddRepositoriesToInstallation(ctx, "\n", "\n", -1, input)
112+
_, _, err = client.Enterprise.AddRepositoriesToInstallation(ctx, "\n", "\n", -1, input)
113113
return err
114114
})
115115

116116
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
117-
_, resp, err := client.EnterpriseApps.AddRepositoriesToInstallation(ctx, "e", "o", 1, input)
117+
_, resp, err := client.Enterprise.AddRepositoriesToInstallation(ctx, "e", "o", 1, input)
118118
return resp, err
119119
})
120120
}
121121

122-
func TestEnterpriseAppsService_RemoveRepositoriesFromInstallation(t *testing.T) {
122+
func TestEnterpriseService_RemoveRepositoriesFromInstallation(t *testing.T) {
123123
t.Parallel()
124124
client, mux, _ := setup(t)
125125

126-
input := &EnterpriseInstallationRepositoriesOptions{SelectedRepositoryIDs: []int64{1, 2}}
126+
input := EnterpriseInstallationRepositoriesOptions{SelectedRepositoryIDs: []int64{1, 2}}
127127

128128
mux.HandleFunc("/enterprises/e/apps/organizations/o/installations/1/repositories/remove", func(w http.ResponseWriter, r *http.Request) {
129129
testMethod(t, r, "PATCH")
@@ -132,24 +132,24 @@ func TestEnterpriseAppsService_RemoveRepositoriesFromInstallation(t *testing.T)
132132
})
133133

134134
ctx := t.Context()
135-
repos, _, err := client.EnterpriseApps.RemoveRepositoriesFromInstallation(ctx, "e", "o", 1, input)
135+
repos, _, err := client.Enterprise.RemoveRepositoriesFromInstallation(ctx, "e", "o", 1, input)
136136
if err != nil {
137-
t.Errorf("EnterpriseApps.RemoveRepositoriesFromInstallation returned error: %v", err)
137+
t.Errorf("Enterprise.RemoveRepositoriesFromInstallation returned error: %v", err)
138138
}
139139

140140
want := &ListRepositories{TotalCount: Ptr(2), Repositories: []*Repository{{ID: Ptr(int64(1))}, {ID: Ptr(int64(2))}}}
141141
if diff := cmp.Diff(repos, want); diff != "" {
142-
t.Errorf("EnterpriseApps.RemoveRepositoriesFromInstallation returned diff (-want +got):\n%v", diff)
142+
t.Errorf("Enterprise.RemoveRepositoriesFromInstallation returned diff (-want +got):\n%v", diff)
143143
}
144144

145145
const methodName = "RemoveRepositoriesFromInstallation"
146146
testBadOptions(t, methodName, func() (err error) {
147-
_, _, err = client.EnterpriseApps.RemoveRepositoriesFromInstallation(ctx, "\n", "\n", -1, input)
147+
_, _, err = client.Enterprise.RemoveRepositoriesFromInstallation(ctx, "\n", "\n", -1, input)
148148
return err
149149
})
150150

151151
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
152-
_, resp, err := client.EnterpriseApps.RemoveRepositoriesFromInstallation(ctx, "e", "o", 1, input)
152+
_, resp, err := client.Enterprise.RemoveRepositoriesFromInstallation(ctx, "e", "o", 1, input)
153153
return resp, err
154154
})
155155
}

‎github/github.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,6 @@ type Client struct {
206206
DependencyGraph *DependencyGraphService
207207
Emojis *EmojisService
208208
Enterprise *EnterpriseService
209-
EnterpriseApps *EnterpriseAppsService
210209
Gists *GistsService
211210
Git *GitService
212211
Gitignores *GitignoresService
@@ -449,7 +448,6 @@ func (c *Client) initialize() {
449448
c.DependencyGraph = (*DependencyGraphService)(&c.common)
450449
c.Emojis = (*EmojisService)(&c.common)
451450
c.Enterprise = (*EnterpriseService)(&c.common)
452-
c.EnterpriseApps = (*EnterpriseAppsService)(&c.common)
453451
c.Gists = (*GistsService)(&c.common)
454452
c.Git = (*GitService)(&c.common)
455453
c.Gitignores = (*GitignoresService)(&c.common)

0 commit comments

Comments
 (0)