Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for terraform component types #515

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Conversation

rocktavious
Copy link
Collaborator

Resolves #

Problem

Connection types were not setup by codegen so this was missed

Solution

Add connection type for property def's on component types

Checklist

  • I have run this code, and it appears to resolve the stated issue.
  • This PR does not reduce total test coverage
  • This PR has no user interface changes or has already received approval from product management to change the interface.
  • Does this change require a Terraform schema change?
    • If so what is the ticket or PR #
  • Make a changie entry that explains the customer facing outcome of this change

@rocktavious rocktavious requested a review from p25marti January 31, 2025 20:51
@rocktavious rocktavious self-assigned this Jan 31, 2025
@@ -14,6 +16,40 @@ type ComponentTypeConnection struct {
TotalCount int `json:"totalCount" graphql:"-"`
}

func (s *ComponentType) GetProperties(client *Client, v *PayloadVariables) (*PropertyDefinitionConnection, error) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the pattern we use for connection types because it limits how much data we pull back when doing "list" api calls.

IsDefault bool // Whether or not the component type is the default (Required)
Name string // The name of the component type (Required)
Timestamps Timestamps // When the component type was created and updated (Required)
Properties *PropertyDefinitionConnection `graphql:"-"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will get to get ported into codegen but this effectively tells the graphql client to ingore this field since we'll populate it with GetProperties when called

@@ -13,7 +13,7 @@ type PropertyDefinition struct {
DisplayType PropertyDefinitionDisplayTypeEnum `graphql:"displayType" json:"displayType"`
PropertyDisplayStatus PropertyDisplayStatusEnum `graphql:"propertyDisplayStatus" json:"propertyDisplayStatus"`
LockedStatus PropertyLockedStatusEnum `graphql:"lockedStatus" json:"lockedStatus"`
Schema JSON `json:"schema" scalar:"true"`
Schema JSONSchema `json:"schema" scalar:"true"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why it was coded like this but in our API its actually this type (this struct isn't under codegen yet)

Copy link
Contributor

@p25marti p25marti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Tested it locally 👍

@rocktavious rocktavious merged commit ecf166c into main Feb 3, 2025
4 checks passed
@rocktavious rocktavious deleted the kr/terraform-component-type branch February 3, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants