fix: improve team_membership deletion logic

[mdb]

This seeks to fix issue #913 by only diassociating/re-associating the team-specific escalation policies for which the user-whose-membership-is-to-be-deleted is a rule target (rather than attempting to diassociate/re-associate all the user-associated escalation policies to the team, as those escalation policies may be taken by other teams).

In other words...

Previously, team membership deletion attempted to diassociate all escalation policies associated to the user from the team, then re-associate those policies back to the team. This could run into the following error if/when that user was a member of multiple teams (see issue #913 for details) and associated to escalation policies associated to other teams:

pagerduty_team_membership.redacted: Still destroying...
[id=REDACTED:REDACTED, 2m0s elapsed]

  | │ Error: PUT API call to
https://api.pagerduty.com/teams/team1/escalation_policies/escalation_policy2
failed 400 Bad Request. Code: 2001, Errors: <nil>, Message: Escalation
Policy has already been taken; Error while trying to associate back team
"team1" to Escalation Policy "escalation_policy2". Resource succesfully
deleted, but some team association couldn't be completed, so you need to
run "terraform plan -refresh-only" and again "terraform apply/destroy"
in order to remediate the drift

This seeks to fix that :)

Result of new acceptance tests introduced...

TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -count=1 -run TestAccPagerDutyTeamMembership -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
?       github.com/PagerDuty/terraform-provider-pagerduty/util/apiutil  [no test files]

=== RUN   TestAccPagerDutyTeamMembership_import
--- PASS: TestAccPagerDutyTeamMembership_import (13.99s)
=== RUN   TestAccPagerDutyTeamMembership_importWithRole
--- PASS: TestAccPagerDutyTeamMembership_importWithRole (14.20s)
=== RUN   TestAccPagerDutyTeamMembership_Basic
--- PASS: TestAccPagerDutyTeamMembership_Basic (13.61s)
=== RUN   TestAccPagerDutyTeamMembership_WithRole
--- PASS: TestAccPagerDutyTeamMembership_WithRole (13.05s)
=== RUN   TestAccPagerDutyTeamMembership_WithRoleConsistentlyAssigned
--- PASS: TestAccPagerDutyTeamMembership_WithRoleConsistentlyAssigned (21.90s)
=== RUN   TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependant
--- PASS: TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependant (21.98s)
=== RUN   TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependantAndMultipleTeams # 👈 New acceptance test 
--- PASS: TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependantAndMultipleTeams (24.00s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     123.264s
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       1.128s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  0.711s [no tests to run]

[mdb]

Full transparency: I don't have a reliable way to run the acceptance tests nor am I intimately familiar with the quirks of the PagerDuty API. So, there could be bugs or mistaken assumptions associated with this logic change, but -- at face value -- I believe this is an improvement over the original logic. Am I mistaken?

[Surmi64]

Are there any chance that this will be merged in the near future? We ran into this problem in the past weeks and it causes a lot of trouble when we would modify any detail of a team with EP.