Add support for setting enterprise properties#3292
Add support for setting enterprise properties#3292jackmtpt wants to merge 5 commits intointegrations:mainfrom
Conversation
|
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with |
|
I'm quite new to this so I'm not really sure how I can get a build and hook up my actual terraform code to use it - do the CI builds of providers in this repo get published to the registry so they can be used for testing? |
|
|
||
| func dataSourceGithubEnterpriseCustomProperties() *schema.Resource { | ||
| return &schema.Resource{ | ||
| ReadContext: dataSourceGithubEnterpriseCustomPropertiesRead, |
There was a problem hiding this comment.
Please add top-level Description field
| "value_type": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| }, | ||
| "required": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, | ||
| }, | ||
| "default_value": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Computed: true, | ||
| }, | ||
| "description": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Computed: true, | ||
| }, | ||
| "allowed_values": { | ||
| Type: schema.TypeList, | ||
| Optional: true, | ||
| Computed: true, | ||
| Elem: &schema.Schema{Type: schema.TypeString}, | ||
| }, | ||
| "values_editable_by": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Computed: true, | ||
| }, |
There was a problem hiding this comment.
None of these should be Optional as you don't use them in the data fetching
| return diag.Errorf("error querying GitHub custom properties %s: %v", entSlug, err) | ||
| } | ||
|
|
||
| // TODO: Add support for other types of default values |
There was a problem hiding this comment.
Please don't submit PRs with missing implementation
|
|
||
| func TestAccGithubEnterpriseCustomPropertiesDataSource(t *testing.T) { | ||
| randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlpha) | ||
| propertyName := fmt.Sprintf("test-%s", randomID) |
There was a problem hiding this comment.
Please use testResourcePrefix
| testAccConf.enterpriseSlug, | ||
| ) | ||
|
|
||
| check := resource.ComposeTestCheckFunc( |
| "enterprise_slug": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| Description: "The slug of the enterprise to which the custom property belongs", |
There was a problem hiding this comment.
This should have ForceNew as well
| Description: "The description of the custom property", | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
Why would this be computed? I doubt the API has a default value for this field
| } | ||
|
|
||
| d.SetId(*customProperty.PropertyName) | ||
| return resourceGithubEnterpriseCustomPropertiesRead(d, meta) |
There was a problem hiding this comment.
Never call any of the CRUD functions
| _ = d.Set("allowed_values", customProperty.AllowedValues) | ||
| _ = d.Set("default_value", defaultValue) | ||
| _ = d.Set("description", customProperty.Description) | ||
| _ = d.Set("property_name", customProperty.PropertyName) | ||
| _ = d.Set("required", customProperty.Required) | ||
| _ = d.Set("value_type", string(customProperty.ValueType)) | ||
| _ = d.Set("values_editable_by", customProperty.ValuesEditableBy) |
|
|
||
| _, err := client.Enterprise.RemoveCustomProperty(context.Background(), entSlug, d.Get("property_name").(string)) | ||
| if err != nil { | ||
| return err |
3 participants
Resolves #3230
I've basically copied the existing org resource/data source and adjusted it for the enterprise API calls. The tests pass for me and I can see the properties appear briefly in my enterprise while the tests are running.
Before the change?
After the change?
Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!