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

bugfix(scalar.go): return nil rather than empty alias for NewIdentifi… #295

Closed

Conversation

msobkowiak-olx
Copy link

NewIdentifier(string) will always produce a non-empty JSON structure, this will not respect the "omitempty" hint on various inputs which can be reproduced with Terraform Provider when provisioning new system:

resource "opslevel_system" "capability" {
  name        = var.capability_name
  description = var.capability_description
  note        = var.capability_note
}

Which yields:

Error: OpsLevel API Errors:
	- 'id' One of id or alias must be supplied

Changelog

  • modifieds scalar.go with a nil fallback if both Alias/ID are not supplied
  • modifies tests to account for the change

@CLAassistant
Copy link

CLAassistant commented Nov 9, 2023

CLA assistant check
All committers have signed the CLA.

@ghost
Copy link

ghost commented Nov 10, 2023

Thanks a lot for submitting this pull request!

After reading your PR we have realized that IdentifierInput and UserIdentifierInput are essentially broken in our tools, which is causing 'id' One of id or alias must be supplied to be thrown whenever a user tries to unset certain foreign keys on certain resources.

We are working on a permanent fix for the next release. This will involve updating these input types to use omitempty on pointer fields across all of our tools. We cannot include a fix in today's release because there are a lot of dependencies on these types.

@msobkowiak-olx
Copy link
Author

Thanks - you may keep it opened for a bit for now.

@davidbloss
Copy link
Contributor

👋 Hi @msobkowiak-olx, the underlying issue raised here has been fixed. Specifically, the fields in the IdentifierInput struct needed to be pointers and we will be releasing this fix tomorrow.

Thank you for digging into this and bringing this to light 🙌

@davidbloss davidbloss closed this Nov 15, 2023
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.

3 participants