franckverrot / terraform-provider-stripe

A Terraform Provider for Stripe
https://registry.terraform.io/providers/franckverrot/stripe/latest
Mozilla Public License 2.0
242 stars 51 forks source link

add tax rate #26

Closed drselump14 closed 4 years ago

drselump14 commented 4 years ago

Add Tax Rate API

Example:

resource "stripe_tax_rate" "default_tax_rate" {
  display_name = "japan_shohizei"
  inclusive = false
  percentage = 10.0
  active = true
  jurisdiction = "JP"
}
franckverrot commented 4 years ago

thanks, I'll take a look at it this week. One quick question though: if the delete API doesn't work yet, does it mean people will have to do manual operations (some manual updates in the UI along with some state rm) in order to get rid of one tax rate?

If so that's okay, but we need to document it properly somewhere (probably the README.)

Thanks!

drselump14 commented 4 years ago

@franckverrot Thank you for the response.

One quick question though: if the delete API doesn't work yet, does it mean people will have to do manual operations (some manual updates in the UI along with some state rm) in order to get rid of one tax rate?

Unfortunately yes, I've contacted the stripe support team, and they said they don't have any plan yet to implement the deletion API, it has to be done via dashboard page.

I'll put that into README.

franckverrot commented 4 years ago

I can release this weekend, I can move forward with the documentation if you don't have time, don't worry about it, but will wait until then if you'd like to.

Thanks again for this contribution!

drselump14 commented 4 years ago

Hello, Sorry I forgot to push the commit. I've added the information about tax rate in README.

franckverrot commented 4 years ago

I've reworked the commits and manually merged, thanks a lot.

One quick comment: I am return an error if a tax rate has to be dropped, hence forcing a manual state operation. The rationale: I don't feel comfortable un-tracking the resource without mentioning to the user that the remote state still has it (ie: removed from local state, still exists on Stripe.)

Please tell me if it's an acceptable behavior. I'll cut a release ASAP.

drselump14 commented 4 years ago

Thanks. I agree with you, I think it's a good idea to throw the error. However, as far I can remember, terraform will invoke the destroy method even for the update operation. So it will throw error when invoking the update operation. I think it needs a special treatment to handle the error

franckverrot commented 4 years ago

Seems like it's fine here. I tried it again just to make sure and it's fine here. Mind confirming it's also for you? I can release first if it's easier.

drselump14 commented 4 years ago

Oh, good then. Probably It's easier if you release it first. I'll try to confirm it later