Skip to content

Commit 18c257c

Browse files
vmvarelavmvarela
committed
fix(billing): address code review issues before upstream PR
- Dereference *string pointer fields (organization_name, repository_name) in flattenUsageItems to avoid storing pointers in TypeString schema fields - Replace deprecated ValidateFunc with ValidateDiagFunc for year/month/day integer validators across all three enterprise billing data sources - Replace fork-local buildID() with upstream-compatible buildTwoPartID() in all three data source Read functions - Update unit test assertions to match dereferenced string values
1 parent af73e8b commit 18c257c

5 files changed

Lines changed: 53 additions & 57 deletions

‎github/data_source_github_enterprise_billing_premium_request_usage.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,22 @@ func dataSourceGithubEnterpriseBillingPremiumRequestUsage() *schema.Resource {
2121
ValidateDiagFunc: validation.ToDiagFunc(validation.StringIsNotEmpty),
2222
},
2323
"year": {
24-
Type: schema.TypeInt,
25-
Optional: true,
26-
Description: "If specified, only return results for a single year.",
27-
ValidateFunc: validation.IntAtLeast(2000),
24+
Type: schema.TypeInt,
25+
Optional: true,
26+
Description: "If specified, only return results for a single year.",
27+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(2000)),
2828
},
2929
"month": {
30-
Type: schema.TypeInt,
31-
Optional: true,
32-
Description: "If specified, only return results for a single month. Value between 1 and 12.",
33-
ValidateFunc: validation.IntBetween(1, 12),
30+
Type: schema.TypeInt,
31+
Optional: true,
32+
Description: "If specified, only return results for a single month. Value between 1 and 12.",
33+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 12)),
3434
},
3535
"day": {
36-
Type: schema.TypeInt,
37-
Optional: true,
38-
Description: "If specified, only return results for a single day. Value between 1 and 31.",
39-
ValidateFunc: validation.IntBetween(1, 31),
36+
Type: schema.TypeInt,
37+
Optional: true,
38+
Description: "If specified, only return results for a single day. Value between 1 and 31.",
39+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 31)),
4040
},
4141
"organization": {
4242
Type: schema.TypeString,
@@ -195,11 +195,7 @@ func dataSourceGithubEnterpriseBillingPremiumRequestUsageRead(ctx context.Contex
195195
return diag.Errorf("error getting enterprise billing premium request usage for %q: %s", enterpriseSlug, err)
196196
}
197197

198-
id, err := buildID(enterpriseSlug, "billing-premium-request-usage")
199-
if err != nil {
200-
return diag.FromErr(err)
201-
}
202-
d.SetId(id)
198+
d.SetId(buildTwoPartID(enterpriseSlug, "billing-premium-request-usage"))
203199

204200
if err := d.Set("time_period", flattenTimePeriod(report.TimePeriod)); err != nil {
205201
return diag.FromErr(err)

‎github/data_source_github_enterprise_billing_usage.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,22 @@ func dataSourceGithubEnterpriseBillingUsage() *schema.Resource {
2121
ValidateDiagFunc: validation.ToDiagFunc(validation.StringIsNotEmpty),
2222
},
2323
"year": {
24-
Type: schema.TypeInt,
25-
Optional: true,
26-
Description: "If specified, only return results for a single year.",
27-
ValidateFunc: validation.IntAtLeast(2000),
24+
Type: schema.TypeInt,
25+
Optional: true,
26+
Description: "If specified, only return results for a single year.",
27+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(2000)),
2828
},
2929
"month": {
30-
Type: schema.TypeInt,
31-
Optional: true,
32-
Description: "If specified, only return results for a single month. Value between 1 and 12.",
33-
ValidateFunc: validation.IntBetween(1, 12),
30+
Type: schema.TypeInt,
31+
Optional: true,
32+
Description: "If specified, only return results for a single month. Value between 1 and 12.",
33+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 12)),
3434
},
3535
"day": {
36-
Type: schema.TypeInt,
37-
Optional: true,
38-
Description: "If specified, only return results for a single day. Value between 1 and 31.",
39-
ValidateFunc: validation.IntBetween(1, 31),
36+
Type: schema.TypeInt,
37+
Optional: true,
38+
Description: "If specified, only return results for a single day. Value between 1 and 31.",
39+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 31)),
4040
},
4141
"cost_center_id": {
4242
Type: schema.TypeString,
@@ -134,11 +134,7 @@ func dataSourceGithubEnterpriseBillingUsageRead(ctx context.Context, d *schema.R
134134
return diag.Errorf("error getting enterprise billing usage for %q: %s", enterpriseSlug, err)
135135
}
136136

137-
id, err := buildID(enterpriseSlug, "billing-usage")
138-
if err != nil {
139-
return diag.FromErr(err)
140-
}
141-
d.SetId(id)
137+
d.SetId(buildTwoPartID(enterpriseSlug, "billing-usage"))
142138

143139
if err := d.Set("usage_items", flattenUsageItems(report.UsageItems)); err != nil {
144140
return diag.FromErr(err)

‎github/data_source_github_enterprise_billing_usage_summary.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,22 @@ func dataSourceGithubEnterpriseBillingUsageSummary() *schema.Resource {
2121
ValidateDiagFunc: validation.ToDiagFunc(validation.StringIsNotEmpty),
2222
},
2323
"year": {
24-
Type: schema.TypeInt,
25-
Optional: true,
26-
Description: "If specified, only return results for a single year.",
27-
ValidateFunc: validation.IntAtLeast(2000),
24+
Type: schema.TypeInt,
25+
Optional: true,
26+
Description: "If specified, only return results for a single year.",
27+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(2000)),
2828
},
2929
"month": {
30-
Type: schema.TypeInt,
31-
Optional: true,
32-
Description: "If specified, only return results for a single month. Value between 1 and 12.",
33-
ValidateFunc: validation.IntBetween(1, 12),
30+
Type: schema.TypeInt,
31+
Optional: true,
32+
Description: "If specified, only return results for a single month. Value between 1 and 12.",
33+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 12)),
3434
},
3535
"day": {
36-
Type: schema.TypeInt,
37-
Optional: true,
38-
Description: "If specified, only return results for a single day. Value between 1 and 31.",
39-
ValidateFunc: validation.IntBetween(1, 31),
36+
Type: schema.TypeInt,
37+
Optional: true,
38+
Description: "If specified, only return results for a single day. Value between 1 and 31.",
39+
ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 31)),
4040
},
4141
"organization": {
4242
Type: schema.TypeString,
@@ -190,11 +190,7 @@ func dataSourceGithubEnterpriseBillingUsageSummaryRead(ctx context.Context, d *s
190190
return diag.Errorf("error getting enterprise billing usage summary for %q: %s", enterpriseSlug, err)
191191
}
192192

193-
id, err := buildID(enterpriseSlug, "billing-usage-summary")
194-
if err != nil {
195-
return diag.FromErr(err)
196-
}
197-
d.SetId(id)
193+
d.SetId(buildTwoPartID(enterpriseSlug, "billing-usage-summary"))
198194

199195
if err := d.Set("time_period", flattenTimePeriod(report.TimePeriod)); err != nil {
200196
return diag.FromErr(err)

‎github/util_enterprise_billing.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,14 @@ func getEnterpriseUsageSummary(ctx context.Context, client *github.Client, enter
236236
func flattenUsageItems(items []*github.UsageItem) []map[string]any {
237237
result := make([]map[string]any, len(items))
238238
for idx, item := range items {
239+
orgName := ""
240+
if item.OrganizationName != nil {
241+
orgName = *item.OrganizationName
242+
}
243+
repoName := ""
244+
if item.RepositoryName != nil {
245+
repoName = *item.RepositoryName
246+
}
239247
result[idx] = map[string]any{
240248
"date": item.Date,
241249
"product": item.Product,
@@ -246,8 +254,8 @@ func flattenUsageItems(items []*github.UsageItem) []map[string]any {
246254
"gross_amount": item.GrossAmount,
247255
"discount_amount": item.DiscountAmount,
248256
"net_amount": item.NetAmount,
249-
"organization_name": item.OrganizationName,
250-
"repository_name": item.RepositoryName,
257+
"organization_name": orgName,
258+
"repository_name": repoName,
251259
}
252260
}
253261
return result

‎github/util_enterprise_billing_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ func TestFlattenUsageItems(t *testing.T) {
108108
assert.InDelta(t, 0.8, result[0]["gross_amount"], 0.0001)
109109
assert.InDelta(t, 0.0, result[0]["discount_amount"], 0.0001)
110110
assert.InDelta(t, 0.8, result[0]["net_amount"], 0.0001)
111-
assert.Equal(t, github.Ptr("test-org"), result[0]["organization_name"])
112-
assert.Equal(t, github.Ptr("test-org/example"), result[0]["repository_name"])
111+
assert.Equal(t, "test-org", result[0]["organization_name"])
112+
assert.Equal(t, "test-org/example", result[0]["repository_name"])
113113
})
114114

115115
t.Run("flattens items with nil optional fields", func(t *testing.T) {
@@ -125,8 +125,8 @@ func TestFlattenUsageItems(t *testing.T) {
125125

126126
result := flattenUsageItems(items)
127127
assert.Len(t, result, 1)
128-
assert.Nil(t, result[0]["organization_name"])
129-
assert.Nil(t, result[0]["repository_name"])
128+
assert.Equal(t, "", result[0]["organization_name"])
129+
assert.Equal(t, "", result[0]["repository_name"])
130130
})
131131
}
132132

0 commit comments

Comments
 (0)