fix: improve github_organization_custom_properties resource#3234
fix: improve github_organization_custom_properties resource#3234mkushakov wants to merge 3 commits into
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 |
| Create: resourceGithubCustomPropertiesCreate, | ||
| Read: resourceGithubCustomPropertiesRead, | ||
| Update: resourceGithubCustomPropertiesUpdate, | ||
| Delete: resourceGithubCustomPropertiesDelete, | ||
| Importer: &schema.ResourceImporter{ | ||
| State: resourceGithubCustomPropertiesImport, |
There was a problem hiding this comment.
Please refactor to use the Context functions
| } | ||
| return resourceGithubCustomPropertiesRead(d, meta) | ||
| // Create uses the same upsert API, and already calls Read at the end | ||
| return resourceGithubCustomPropertiesCreate(d, meta) |
There was a problem hiding this comment.
issue: Never call any CRUD function
mkushakov
force-pushed
the
fix/org-custom-properties-improvements
branch
from
c8c34c4 to
b4fdf7b
Compare
mkushakov
force-pushed
the
fix/org-custom-properties-improvements
branch
from
a5054a9 to
1267efc
Compare
- Remove broken CustomizeDiff referencing non-existent 'slug' field - Make 'property_name' ForceNew (renaming creates a new property, orphans old) - Make 'value_type' Required and ForceNew (can't create without type, changing type requires recreation) - Add allowed_values validation: required for select types, rejected for others - Add checkOrganization guard to all CRUD functions - Add graceful 404 handling in Read (removes from state instead of erroring) - Improve error messages with context - Fix Update double-read (Create already calls Read at the end) - Add resource Description - Use GetOk for optional fields to avoid sending zero values Resolves integrations#2806
mkushakov
force-pushed
the
fix/org-custom-properties-improvements
branch
from
1267efc to
e40236c
Compare
|
@mkushakov if you're still interested in contributing this change, I think it may be better to create a new resource named correctly (#2936) and then deprecate this resource. Off the top of my head I think the resource should be I've done a full refactor on the |
3 participants
Summary
Fix several bugs and improve the
github_organization_custom_propertiesresource to handle edge cases correctly and align with GitHub API behavior.Resolves #2806, #3191
Changes
Bug Fixes
CustomizeDiff: The existingCustomizeDiffreferenced aslugfield that does not exist in the schema, causing unnecessary plan diffs. Removed entirely.GetOkfor optional fields: Changedallowed_valuesanddefault_valuereads to useGetOkinstead ofGetto correctly distinguish between unset and empty values.Improvements
ForceNewto immutable fields:property_nameandvalue_typecannot be changed on an existing custom property — they now force recreation.value_typeRequired: This field is required by the GitHub API and should not be optional.allowed_valuesvalidation:allowed_valuesis now required whenvalue_typeissingle_selectormulti_select, and rejected forstringandtrue_falsetypes viaCustomizeDiff.checkOrganizationguard: Create/Read/Update/Delete now validate that an organization is configured.fmt.Errorffor better context.Tests
allowed_valuesforstringtype.allowed_valuesforsingle_selecttype.Testing
go build ./...— passesgo vet ./...— passes