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

New TaxRates are being created by ignoring existing TaxRates #462

Open
sebin-vincent-ayata opened this issue Dec 12, 2023 · 2 comments · May be fixed by #477
Open

New TaxRates are being created by ignoring existing TaxRates #462

sebin-vincent-ayata opened this issue Dec 12, 2023 · 2 comments · May be fixed by #477
Assignees
Labels
bug Something isn't working

Comments

@sebin-vincent-ayata
Copy link

sebin-vincent-ayata commented Dec 12, 2023

It came to our attention that when any updates are made on tax rates within a tax category manually, CommerceTools (CT) changes the ID of the Tax Rate itself. Consequently, when Terraform attempts to locate a tax category rate using the saved ID in it's statefile, discrepancies arise due to the manual update of taxRates. As a result, Terraform attempts to create a new tax category rate because as far as terraform is concerned the existing tax category rate got deleted somehow. You can view a demonstration of this scenario in the following video [link].

I checked this with commercetools support to clarify if this is an expected behaviour from commercetools or is it a bug. According to them, this is an expected behaviour because this is actually a replace action rather than update action so it is expected to have a change in the Id.
image

Their recommendation is to use Tax Rate Key instead of Tax Rate Id, as the earlier remains same during a Tax Rate update.
image

@sebin-vincent-ayata sebin-vincent-ayata added bug Something isn't working triage Needs triage labels Dec 12, 2023
@demeyerthom
Copy link
Member

Hi @sebin-vincent-ayata thanks for reaching out!

Hmm, thanks for looking into that. That is indeed not the behaviour we would want here. However I am also loathe to switch to using the key as identifier here because this would conflict with how we treat all the other resources. This might introduce bugs down the line if we ever refer to tax rates by id elsewhere.

I will do some testing and see If I can come up with a different solution

@demeyerthom demeyerthom removed the triage Needs triage label Dec 15, 2023
@demeyerthom demeyerthom self-assigned this Dec 15, 2023
@demeyerthom
Copy link
Member

@sebin-vincent-ayata I did some research into this, and I came to the conclusion that using the key is probably not the best way of doing this. The key is optional in commercetools, so we run the risk that this gets removed manually, which is equally a bug-risk. I also don't want to fill in the key for the user with something the provider can understand, as developers might use the key to have a stable reference to use in their code (which is the intention for this field).

So my alternative solution is to implement #291, and see if I can ensure a smooth migration between the old system and the new. This keeps the data model closer to the commercetools API, and we can do the required checks internally in the resource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants