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

✨ Allow empty site description #314

Merged
merged 1 commit into from
Dec 22, 2022

Conversation

keshy7
Copy link

@keshy7 keshy7 commented Dec 19, 2022

Currently, site that has been previously configured with a description can no longer be cleared out. Even if the terraform configuration indicates empty or nil description, the change will not be persisted to NSOT. Additionally, every time a terraform plan is run, it will always detect an update-in-place for this field.

This change fixes this behavior by setting site description's to space character. This is based on the pattern on other existing resource such as network device.

References:

@keshy7 keshy7 force-pushed the AllowEmptySiteDescription branch from dd50f0d to 89d522d Compare December 19, 2022 06:08
@fbreckle
Copy link
Collaborator

Hi,

can you please add a test for this fix? I.e. add the description attribute to the tests and remove it in a later step. The testing framework then detects non-empty plans. To test locally, you can just temporarily comment your fix. Your test should then fail. Then uncomment the fix and it should pass.

Currently, site that has been previously configured with a description
can no longer be cleared out. Even if the terraform configuration
indicates empty or nil description, the change will not be persisted to
NSOT. Additionally, every time a terraform plan is run, it will always
detect an update-in-place for this field.

This change fixes this behavior by setting site description's to
space character. This is based on the pattern on other existing resource
such as network device.

Also added TestAccNetboxSite_fieldUpdate() test to test updates on
fields.

References:
- fbreckle/go-netbox#25
- https://github.com/e-breuninger/terraform-provider-netbox/blob/master/netbox/resource_netbox_device.go#L331
@keshy7 keshy7 force-pushed the AllowEmptySiteDescription branch from 89d522d to b0f8560 Compare December 20, 2022 02:00
@keshy7
Copy link
Author

keshy7 commented Dec 20, 2022

Thank you for this check, @fbreckle! I found an issue in my previous code because of the test. The changes are now in. Thank you once again for your kind direction.

@fbreckle fbreckle merged commit 9e3e1d9 into e-breuninger:master Dec 22, 2022
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