Skip to content

Commit 5844ab3

Browse files
committed
fix: Address maintainer review feedback on enterprise custom property resources
Switch to context-aware CRUD functions, add 404 handling in delete, properly propagate d.Set() errors, add top-level Description to data source, fix data source schema to use Computed-only fields, and update tests to use testResourcePrefix and ConfigStateChecks.
1 parent 4b4d1fb commit 5844ab3

File tree

3 files changed

+126
-123
lines changed

3 files changed

+126
-123
lines changed

github/data_source_github_enterprise_custom_property.go

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99

1010
func dataSourceGithubEnterpriseCustomProperty() *schema.Resource {
1111
return &schema.Resource{
12+
Description: "Use this data source to retrieve information about a custom property definition for a GitHub enterprise.",
13+
1214
ReadContext: dataSourceGithubEnterpriseCustomPropertyRead,
1315

1416
Schema: map[string]*schema.Schema{
@@ -71,14 +73,31 @@ func dataSourceGithubEnterpriseCustomPropertyRead(ctx context.Context, d *schema
7173
defaultValue, _ := property.DefaultValueString()
7274

7375
d.SetId(buildTwoPartID(enterpriseSlug, propertyName))
74-
_ = d.Set("enterprise_slug", enterpriseSlug)
75-
_ = d.Set("property_name", property.GetPropertyName())
76-
_ = d.Set("value_type", string(property.ValueType))
77-
_ = d.Set("required", property.GetRequired())
78-
_ = d.Set("default_value", defaultValue)
79-
_ = d.Set("description", property.GetDescription())
80-
_ = d.Set("allowed_values", property.AllowedValues)
81-
_ = d.Set("values_editable_by", property.GetValuesEditableBy())
76+
77+
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
78+
return diag.FromErr(err)
79+
}
80+
if err := d.Set("property_name", property.GetPropertyName()); err != nil {
81+
return diag.FromErr(err)
82+
}
83+
if err := d.Set("value_type", string(property.ValueType)); err != nil {
84+
return diag.FromErr(err)
85+
}
86+
if err := d.Set("required", property.GetRequired()); err != nil {
87+
return diag.FromErr(err)
88+
}
89+
if err := d.Set("default_value", defaultValue); err != nil {
90+
return diag.FromErr(err)
91+
}
92+
if err := d.Set("description", property.GetDescription()); err != nil {
93+
return diag.FromErr(err)
94+
}
95+
if err := d.Set("allowed_values", property.AllowedValues); err != nil {
96+
return diag.FromErr(err)
97+
}
98+
if err := d.Set("values_editable_by", property.GetValuesEditableBy()); err != nil {
99+
return diag.FromErr(err)
100+
}
82101

83102
return nil
84103
}

github/resource_github_enterprise_custom_properties.go

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,19 @@ import (
66
"net/http"
77

88
"github.com/google/go-github/v84/github"
9+
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
910
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1011
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1112
)
1213

1314
func resourceGithubEnterpriseCustomProperties() *schema.Resource {
1415
return &schema.Resource{
15-
Create: resourceGithubEnterpriseCustomPropertiesCreate,
16-
Read: resourceGithubEnterpriseCustomPropertiesRead,
17-
Update: resourceGithubEnterpriseCustomPropertiesUpdate,
18-
Delete: resourceGithubEnterpriseCustomPropertiesDelete,
16+
CreateContext: resourceGithubEnterpriseCustomPropertiesCreate,
17+
ReadContext: resourceGithubEnterpriseCustomPropertiesRead,
18+
UpdateContext: resourceGithubEnterpriseCustomPropertiesUpdate,
19+
DeleteContext: resourceGithubEnterpriseCustomPropertiesDelete,
1920
Importer: &schema.ResourceImporter{
20-
State: resourceGithubEnterpriseCustomPropertiesImport,
21+
StateContext: schema.ImportStatePassthroughContext,
2122
},
2223

2324
Schema: map[string]*schema.Schema{
@@ -53,7 +54,6 @@ func resourceGithubEnterpriseCustomProperties() *schema.Resource {
5354
"description": {
5455
Type: schema.TypeString,
5556
Optional: true,
56-
Computed: true,
5757
Description: "A short description of the custom property.",
5858
},
5959
"allowed_values": {
@@ -74,9 +74,8 @@ func resourceGithubEnterpriseCustomProperties() *schema.Resource {
7474
}
7575
}
7676

77-
func resourceGithubEnterpriseCustomPropertiesCreate(d *schema.ResourceData, meta any) error {
77+
func resourceGithubEnterpriseCustomPropertiesCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
7878
client := meta.(*Owner).v3client
79-
ctx := context.Background()
8079

8180
enterpriseSlug := d.Get("enterprise_slug").(string)
8281
propertyName := d.Get("property_name").(string)
@@ -85,20 +84,19 @@ func resourceGithubEnterpriseCustomPropertiesCreate(d *schema.ResourceData, meta
8584

8685
_, _, err := client.Enterprise.CreateOrUpdateCustomProperty(ctx, enterpriseSlug, propertyName, property)
8786
if err != nil {
88-
return err
87+
return diag.FromErr(err)
8988
}
9089

9190
d.SetId(buildTwoPartID(enterpriseSlug, propertyName))
92-
return resourceGithubEnterpriseCustomPropertiesRead(d, meta)
91+
return resourceGithubEnterpriseCustomPropertiesRead(ctx, d, meta)
9392
}
9493

95-
func resourceGithubEnterpriseCustomPropertiesRead(d *schema.ResourceData, meta any) error {
94+
func resourceGithubEnterpriseCustomPropertiesRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
9695
client := meta.(*Owner).v3client
97-
ctx := context.Background()
9896

9997
enterpriseSlug, propertyName, err := parseTwoPartID(d.Id(), "enterprise_slug", "property_name")
10098
if err != nil {
101-
return err
99+
return diag.FromErr(err)
102100
}
103101

104102
property, resp, err := client.Enterprise.GetCustomProperty(ctx, enterpriseSlug, propertyName)
@@ -108,54 +106,74 @@ func resourceGithubEnterpriseCustomPropertiesRead(d *schema.ResourceData, meta a
108106
d.SetId("")
109107
return nil
110108
}
111-
return err
109+
return diag.FromErr(err)
112110
}
113111

114112
defaultValue, _ := property.DefaultValueString()
115113

116-
d.SetId(buildTwoPartID(enterpriseSlug, propertyName))
117-
_ = d.Set("enterprise_slug", enterpriseSlug)
118-
_ = d.Set("property_name", property.GetPropertyName())
119-
_ = d.Set("value_type", string(property.ValueType))
120-
_ = d.Set("required", property.GetRequired())
121-
_ = d.Set("default_value", defaultValue)
122-
_ = d.Set("description", property.GetDescription())
123-
_ = d.Set("allowed_values", property.AllowedValues)
124-
_ = d.Set("values_editable_by", property.GetValuesEditableBy())
114+
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
115+
return diag.FromErr(err)
116+
}
117+
if err := d.Set("property_name", property.GetPropertyName()); err != nil {
118+
return diag.FromErr(err)
119+
}
120+
if err := d.Set("value_type", string(property.ValueType)); err != nil {
121+
return diag.FromErr(err)
122+
}
123+
if err := d.Set("required", property.GetRequired()); err != nil {
124+
return diag.FromErr(err)
125+
}
126+
if err := d.Set("default_value", defaultValue); err != nil {
127+
return diag.FromErr(err)
128+
}
129+
if err := d.Set("description", property.GetDescription()); err != nil {
130+
return diag.FromErr(err)
131+
}
132+
if err := d.Set("allowed_values", property.AllowedValues); err != nil {
133+
return diag.FromErr(err)
134+
}
135+
if err := d.Set("values_editable_by", property.GetValuesEditableBy()); err != nil {
136+
return diag.FromErr(err)
137+
}
125138

126139
return nil
127140
}
128141

129-
func resourceGithubEnterpriseCustomPropertiesUpdate(d *schema.ResourceData, meta any) error {
142+
func resourceGithubEnterpriseCustomPropertiesUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
130143
client := meta.(*Owner).v3client
131-
ctx := context.Background()
132144

133145
enterpriseSlug, propertyName, err := parseTwoPartID(d.Id(), "enterprise_slug", "property_name")
134146
if err != nil {
135-
return err
147+
return diag.FromErr(err)
136148
}
137149

138150
property := buildEnterpriseCustomProperty(d)
139151

140152
_, _, err = client.Enterprise.CreateOrUpdateCustomProperty(ctx, enterpriseSlug, propertyName, property)
141153
if err != nil {
142-
return err
154+
return diag.FromErr(err)
143155
}
144156

145-
return resourceGithubEnterpriseCustomPropertiesRead(d, meta)
157+
return resourceGithubEnterpriseCustomPropertiesRead(ctx, d, meta)
146158
}
147159

148-
func resourceGithubEnterpriseCustomPropertiesDelete(d *schema.ResourceData, meta any) error {
160+
func resourceGithubEnterpriseCustomPropertiesDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
149161
client := meta.(*Owner).v3client
150-
ctx := context.Background()
151162

152163
enterpriseSlug, propertyName, err := parseTwoPartID(d.Id(), "enterprise_slug", "property_name")
153164
if err != nil {
154-
return err
165+
return diag.FromErr(err)
155166
}
156167

157-
_, err = client.Enterprise.RemoveCustomProperty(ctx, enterpriseSlug, propertyName)
158-
return err
168+
resp, err := client.Enterprise.RemoveCustomProperty(ctx, enterpriseSlug, propertyName)
169+
if err != nil {
170+
if resp != nil && resp.StatusCode == http.StatusNotFound {
171+
return nil
172+
}
173+
return diag.FromErr(err)
174+
}
175+
176+
return nil
159177
}
160178

161179
func resourceGithubEnterpriseCustomPropertiesImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
@@ -164,8 +182,12 @@ func resourceGithubEnterpriseCustomPropertiesImport(d *schema.ResourceData, meta
164182
return nil, err
165183
}
166184

167-
_ = d.Set("enterprise_slug", enterpriseSlug)
168-
_ = d.Set("property_name", propertyName)
185+
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
186+
return nil, err
187+
}
188+
if err := d.Set("property_name", propertyName); err != nil {
189+
return nil, err
190+
}
169191

170192
return []*schema.ResourceData{d}, nil
171193
}

0 commit comments

Comments
 (0)