onelogin / terraform-provider-onelogin

GNU General Public License v3.0
27 stars 19 forks source link

[Bugfix] add parameter handling on role updating #27

Closed at-k closed 3 years ago

at-k commented 3 years ago

I found a bug on role updating procedure. Can you review this PR ?

Problem

error occurs when updating roles example

  # onelogin_roles.role_test_2 will be updated in-place
  ~ resource "onelogin_roles" "role_test_2" {
        admins = []
        apps   = []
        id     = "387061"
        name   = "Test Role"
      ~ users  = [
          - 113470435,
        ]
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

onelogin_roles.role_test_2: Modifying... [id=387061]

Error: No ID Given

  on main.tf line 31, in resource "onelogin_roles" "role_test_2":
  31: resource onelogin_roles role_test_2 {

Solution

This happens because of lack of parameter checked here.

This PR introduce same approach implemented here

dcaponi commented 3 years ago

Hello @at-k thank you for looking into this and fix it. I admit this was an oversight on my part and agree with your changes. Please address the comments I left here and I'll merge your PR 😄

at-k commented 3 years ago

@dcaponi thank you for your review. I adopt the idea you pointed out. Can you review again?

dcaponi commented 3 years ago

@at-k Thank you again so much for your contribution. I'll merge your PR. Feel free to reach out again if you have any more feedback and I look forward to working with you again 😄