fix: Fix empty value check to handle default numeric values correctly #286
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on #283, I discovered that optional numeric fields with
0
as a default value are skipped even though they appear in the planned changes. That also takes place when we use0
in the configuration, and the related field either has no default value or has0
in there:The problem in the provider's code is that
HasChange
returnsfalse
when the default is0
:terraform-provider-routeros/routeros/mikrotik_serialize.go
Line 51 in 678c9a4
After drilling down into the origin of this problem, I found out that terraform SDK casts
null
to0
, and therefore,HasChange
compares two zeros.hashicorp/terraform-plugin-sdk#817 (comment) explained the nature of such behavior and convinced me that it doesn't make much sense to try to fix that on the SDK side. So, I slightly changed the empty check logic not to skip such values.
Unfortunately, I could not cover that with a unit test in
mikrotik_serialize_test.go
because it uses theGetRawConfig
method, which always returns a null value when using eitherTestResourceData
orTestResourceDataRaw
. And I couldn't find a clean way to set the raw config in the resource data state.