Skip to content

Commit e1aaa10

Browse files
vmvarelavmvarela
committed
fix(enterprise-scim): address review findings
- Rename single-char variables (Rule #29): m→member, e→email, r→role, u→user, g→group - Fix descriptions (Rule #2a): 'Lookup' → 'Retrieves' in all 4 data sources - Fix ValidateFunc → ValidateDiagFunc (Rule #23) in users/groups - Migrate tests from Check to ConfigStateChecks (Rule #31) - Add unit tests for all flatten utility functions (Rule #33) - Add sidebar entries for SCIM data sources in github.erb
1 parent 74b9b4d commit e1aaa10

11 files changed

+456
-60
lines changed

‎github/data_source_github_enterprise_scim_group.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func dataSourceGithubEnterpriseSCIMGroup() *schema.Resource {
2222
}
2323

2424
return &schema.Resource{
25-
Description: "Lookup SCIM provisioning information for a single GitHub enterprise group.",
25+
Description: "Retrieves SCIM provisioning information for a single GitHub enterprise group.",
2626
ReadContext: dataSourceGithubEnterpriseSCIMGroupRead,
2727
Schema: s,
2828
}

‎github/data_source_github_enterprise_scim_group_test.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"testing"
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
8+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
9+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
10+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
811
)
912

1013
func TestAccGithubEnterpriseSCIMGroupDataSource(t *testing.T) {
@@ -23,11 +26,10 @@ func TestAccGithubEnterpriseSCIMGroupDataSource(t *testing.T) {
2326
scim_group_id = data.github_enterprise_scim_groups.all.resources[0].id
2427
}
2528
`, testAccConf.enterpriseSlug),
26-
Check: resource.ComposeTestCheckFunc(
27-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_group.test", "id"),
28-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_group.test", "display_name"),
29-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_group.test", "schemas.#"),
30-
),
29+
ConfigStateChecks: []statecheck.StateCheck{
30+
statecheck.ExpectKnownValue("data.github_enterprise_scim_group.test", tfjsonpath.New("display_name"), knownvalue.NotNull()),
31+
statecheck.ExpectKnownValue("data.github_enterprise_scim_group.test", tfjsonpath.New("schemas"), knownvalue.NotNull()),
32+
},
3133
}},
3234
})
3335
})

‎github/data_source_github_enterprise_scim_groups.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
func dataSourceGithubEnterpriseSCIMGroups() *schema.Resource {
1414
return &schema.Resource{
15-
Description: "Lookup SCIM groups provisioned for a GitHub enterprise.",
15+
Description: "Retrieves SCIM groups provisioned for a GitHub enterprise.",
1616
ReadContext: dataSourceGithubEnterpriseSCIMGroupsRead,
1717

1818
Schema: map[string]*schema.Schema{
@@ -27,11 +27,11 @@ func dataSourceGithubEnterpriseSCIMGroups() *schema.Resource {
2727
Optional: true,
2828
},
2929
"results_per_page": {
30-
Description: "Number of results per request (mapped to SCIM 'count'). Used while auto-fetching all pages.",
31-
Type: schema.TypeInt,
32-
Optional: true,
33-
Default: 100,
34-
ValidateFunc: validation.IntBetween(1, 100),
30+
Description: "Number of results per request (mapped to SCIM 'count'). Used while auto-fetching all pages.",
31+
Type: schema.TypeInt,
32+
Optional: true,
33+
Default: 100,
34+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 100)),
3535
},
3636

3737
"schemas": {
@@ -80,8 +80,8 @@ func dataSourceGithubEnterpriseSCIMGroupsRead(ctx context.Context, d *schema.Res
8080
}
8181

8282
flat := make([]any, 0, len(groups))
83-
for _, g := range groups {
84-
flat = append(flat, flattenEnterpriseSCIMGroup(g))
83+
for _, group := range groups {
84+
flat = append(flat, flattenEnterpriseSCIMGroup(group))
8585
}
8686

8787
id := fmt.Sprintf("%s/scim-groups", enterprise)

‎github/data_source_github_enterprise_scim_groups_test.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"testing"
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
8+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
9+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
10+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
811
)
912

1013
func TestAccGithubEnterpriseSCIMGroupsDataSource(t *testing.T) {
@@ -18,12 +21,11 @@ func TestAccGithubEnterpriseSCIMGroupsDataSource(t *testing.T) {
1821
enterprise = "%s"
1922
}
2023
`, testAccConf.enterpriseSlug),
21-
Check: resource.ComposeTestCheckFunc(
22-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_groups.test", "id"),
23-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_groups.test", "total_results"),
24-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_groups.test", "schemas.#"),
25-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_groups.test", "resources.#"),
26-
),
24+
ConfigStateChecks: []statecheck.StateCheck{
25+
statecheck.ExpectKnownValue("data.github_enterprise_scim_groups.test", tfjsonpath.New("total_results"), knownvalue.NotNull()),
26+
statecheck.ExpectKnownValue("data.github_enterprise_scim_groups.test", tfjsonpath.New("schemas"), knownvalue.NotNull()),
27+
statecheck.ExpectKnownValue("data.github_enterprise_scim_groups.test", tfjsonpath.New("resources"), knownvalue.NotNull()),
28+
},
2729
}},
2830
})
2931
})

‎github/data_source_github_enterprise_scim_user.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func dataSourceGithubEnterpriseSCIMUser() *schema.Resource {
2222
}
2323

2424
return &schema.Resource{
25-
Description: "Lookup SCIM provisioning information for a single GitHub enterprise user.",
25+
Description: "Retrieves SCIM provisioning information for a single GitHub enterprise user.",
2626
ReadContext: dataSourceGithubEnterpriseSCIMUserRead,
2727
Schema: s,
2828
}

‎github/data_source_github_enterprise_scim_user_test.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"testing"
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
8+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
9+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
10+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
811
)
912

1013
func TestAccGithubEnterpriseSCIMUserDataSource(t *testing.T) {
@@ -23,13 +26,12 @@ func TestAccGithubEnterpriseSCIMUserDataSource(t *testing.T) {
2326
scim_user_id = data.github_enterprise_scim_users.all.resources[0].id
2427
}
2528
`, testAccConf.enterpriseSlug),
26-
Check: resource.ComposeTestCheckFunc(
27-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_user.test", "id"),
28-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_user.test", "user_name"),
29-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_user.test", "display_name"),
30-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_user.test", "schemas.#"),
31-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_user.test", "emails.#"),
32-
),
29+
ConfigStateChecks: []statecheck.StateCheck{
30+
statecheck.ExpectKnownValue("data.github_enterprise_scim_user.test", tfjsonpath.New("user_name"), knownvalue.NotNull()),
31+
statecheck.ExpectKnownValue("data.github_enterprise_scim_user.test", tfjsonpath.New("display_name"), knownvalue.NotNull()),
32+
statecheck.ExpectKnownValue("data.github_enterprise_scim_user.test", tfjsonpath.New("schemas"), knownvalue.NotNull()),
33+
statecheck.ExpectKnownValue("data.github_enterprise_scim_user.test", tfjsonpath.New("emails"), knownvalue.NotNull()),
34+
},
3335
}},
3436
})
3537
})

‎github/data_source_github_enterprise_scim_users.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
func dataSourceGithubEnterpriseSCIMUsers() *schema.Resource {
1414
return &schema.Resource{
15-
Description: "Lookup SCIM users provisioned for a GitHub enterprise.",
15+
Description: "Retrieves SCIM users provisioned for a GitHub enterprise.",
1616
ReadContext: dataSourceGithubEnterpriseSCIMUsersRead,
1717

1818
Schema: map[string]*schema.Schema{
@@ -27,11 +27,11 @@ func dataSourceGithubEnterpriseSCIMUsers() *schema.Resource {
2727
Optional: true,
2828
},
2929
"results_per_page": {
30-
Description: "Number of results per request (mapped to SCIM 'count'). Used while auto-fetching all pages.",
31-
Type: schema.TypeInt,
32-
Optional: true,
33-
Default: 100,
34-
ValidateFunc: validation.IntBetween(1, 100),
30+
Description: "Number of results per request (mapped to SCIM 'count'). Used while auto-fetching all pages.",
31+
Type: schema.TypeInt,
32+
Optional: true,
33+
Default: 100,
34+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 100)),
3535
},
3636

3737
"schemas": {
@@ -80,8 +80,8 @@ func dataSourceGithubEnterpriseSCIMUsersRead(ctx context.Context, d *schema.Reso
8080
}
8181

8282
flat := make([]any, 0, len(users))
83-
for _, u := range users {
84-
flat = append(flat, flattenEnterpriseSCIMUser(u))
83+
for _, user := range users {
84+
flat = append(flat, flattenEnterpriseSCIMUser(user))
8585
}
8686

8787
id := fmt.Sprintf("%s/scim-users", enterprise)

‎github/data_source_github_enterprise_scim_users_test.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"testing"
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
8+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
9+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
10+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
811
)
912

1013
func TestAccGithubEnterpriseSCIMUsersDataSource(t *testing.T) {
@@ -18,12 +21,11 @@ func TestAccGithubEnterpriseSCIMUsersDataSource(t *testing.T) {
1821
enterprise = "%s"
1922
}
2023
`, testAccConf.enterpriseSlug),
21-
Check: resource.ComposeTestCheckFunc(
22-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_users.test", "id"),
23-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_users.test", "total_results"),
24-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_users.test", "schemas.#"),
25-
resource.TestCheckResourceAttrSet("data.github_enterprise_scim_users.test", "resources.#"),
26-
),
24+
ConfigStateChecks: []statecheck.StateCheck{
25+
statecheck.ExpectKnownValue("data.github_enterprise_scim_users.test", tfjsonpath.New("total_results"), knownvalue.NotNull()),
26+
statecheck.ExpectKnownValue("data.github_enterprise_scim_users.test", tfjsonpath.New("schemas"), knownvalue.NotNull()),
27+
statecheck.ExpectKnownValue("data.github_enterprise_scim_users.test", tfjsonpath.New("resources"), knownvalue.NotNull()),
28+
},
2729
}},
2830
})
2931
})

‎github/util_enterprise_scim.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,15 @@ func flattenEnterpriseSCIMMeta(meta *gh.SCIMEnterpriseMeta) []any {
123123

124124
func flattenEnterpriseSCIMGroupMembers(members []*gh.SCIMEnterpriseDisplayReference) []any {
125125
out := make([]any, 0, len(members))
126-
for _, m := range members {
126+
for _, member := range members {
127127
item := map[string]any{
128-
"value": m.Value,
128+
"value": member.Value,
129129
}
130-
if m.Ref != nil {
131-
item["ref"] = *m.Ref
130+
if member.Ref != nil {
131+
item["ref"] = *member.Ref
132132
}
133-
if m.Display != nil {
134-
item["display_name"] = *m.Display
133+
if member.Display != nil {
134+
item["display_name"] = *member.Display
135135
}
136136
out = append(out, item)
137137
}
@@ -175,30 +175,30 @@ func flattenEnterpriseSCIMUserName(name *gh.SCIMEnterpriseUserName) []any {
175175

176176
func flattenEnterpriseSCIMUserEmails(emails []*gh.SCIMEnterpriseUserEmail) []any {
177177
out := make([]any, 0, len(emails))
178-
for _, e := range emails {
178+
for _, email := range emails {
179179
out = append(out, map[string]any{
180-
"value": e.Value,
181-
"type": e.Type,
182-
"primary": e.Primary,
180+
"value": email.Value,
181+
"type": email.Type,
182+
"primary": email.Primary,
183183
})
184184
}
185185
return out
186186
}
187187

188188
func flattenEnterpriseSCIMUserRoles(roles []*gh.SCIMEnterpriseUserRole) []any {
189189
out := make([]any, 0, len(roles))
190-
for _, r := range roles {
190+
for _, role := range roles {
191191
item := map[string]any{
192-
"value": r.Value,
192+
"value": role.Value,
193193
}
194-
if r.Display != nil {
195-
item["display"] = *r.Display
194+
if role.Display != nil {
195+
item["display"] = *role.Display
196196
}
197-
if r.Type != nil {
198-
item["type"] = *r.Type
197+
if role.Type != nil {
198+
item["type"] = *role.Type
199199
}
200-
if r.Primary != nil {
201-
item["primary"] = *r.Primary
200+
if role.Primary != nil {
201+
item["primary"] = *role.Primary
202202
}
203203
out = append(out, item)
204204
}

0 commit comments

Comments
 (0)