fix: improve team_membership deletion logic

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 :)

pagerduty/resource_pagerduty_team_membership.go

@@ -203,13 +203,14 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac
 
 	log.Printf("[DEBUG] Removing user: %s from team: %s", userID, teamID)
 
-	// Extracting Escalation Policies ids where this team referenced
-	epsAssociatedToUser, err := extractEPsAssociatedToUser(client, userID)
+	// Extract Escalation Policies associated to the team for which the userID is
+	// a rule target.
+	epsAssociatedToTeamAndUser, err := extractEPsAssociatedToTeamAndUser(client, teamID, userID)
 	if err != nil {
 		return err
 	}
 
-	epsDissociatedFromTeam, err := dissociateEPsFromTeam(client, teamID, epsAssociatedToUser)
+	epsDissociatedFromTeam, err := dissociateEPsFromTeam(client, teamID, epsAssociatedToTeamAndUser)
 	if err != nil {
 		return err
 	}
@@ -240,20 +241,21 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac
 	return nil
 }
 
-func buildEPsIdsList(l []*pagerduty.OnCall) []string {
+func buildEPsIdsList(l []*pagerduty.EscalationPolicy) []string {
 	eps := []string{}
 	for _, o := range l {
-		if o.EscalationPolicy != nil {
-			eps = append(eps, o.EscalationPolicy.ID)
-		}
+		eps = append(eps, o.ID)
 	}
 	return unique(eps)
 }
 
-func extractEPsAssociatedToUser(c *pagerduty.Client, userID string) ([]string, error) {
-	var oncalls []*pagerduty.OnCall
+// extractEPsAssociatedToTeamAndUser returns the IDs of escalation policies
+// associated to the specified team and for which the specified user is a rule
+// target.
+func extractEPsAssociatedToTeamAndUser(c *pagerduty.Client, teamID, userID string) ([]string, error) {
+	var eps []*pagerduty.EscalationPolicy
 	retryErr := retry.Retry(2*time.Minute, func() *retry.RetryError {
-		resp, _, err := c.OnCall.List(&pagerduty.ListOnCallOptions{UserIds: []string{userID}})
+		resp, _, err := c.EscalationPolicies.List(&pagerduty.ListEscalationPoliciesOptions{TeamIDs: []string{teamID}})
 		if err != nil {
 			if isErrCode(err, http.StatusBadRequest) {
 				return retry.NonRetryableError(err)
@@ -262,14 +264,28 @@ func extractEPsAssociatedToUser(c *pagerduty.Client, userID string) ([]string, e
 			time.Sleep(2 * time.Second)
 			return retry.RetryableError(err)
 		}
-		oncalls = resp.Oncalls
+		eps = resp.EscalationPolicies
 		return nil
 	})
 	if retryErr != nil {
 		return nil, retryErr
 	}
-	epsAssociatedToUser := buildEPsIdsList(oncalls)
-	return epsAssociatedToUser, nil
+
+	// filter all team escalation policies to only those for which the specified
+	// user is a target.
+	userEPs := []*pagerduty.EscalationPolicy{}
+	for _, ep := range eps {
+		for _, rule := range ep.EscalationRules {
+			for _, target := range rule.Targets {
+				if target.ID == userID {
+					userEPs = append(userEPs, ep)
+				}
+			}
+		}
+	}
+
+	epsAssociatedToTeamAndUser := buildEPsIdsList(userEPs)
+	return epsAssociatedToTeamAndUser, nil
 }
 
 func dissociateEPsFromTeam(c *pagerduty.Client, teamID string, eps []string) ([]string, error) {

pagerduty/resource_pagerduty_team_membership_test.go

@@ -107,6 +107,35 @@ func TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependant(t *test
 	})
 }
 
+func TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependantAndMultipleTeams(t *testing.T) {
+	userOne := fmt.Sprintf("tf-%s", acctest.RandString(5))
+	teamOne := fmt.Sprintf("tf-%s", acctest.RandString(5))
+	teamTwo := fmt.Sprintf("tf-%s", acctest.RandString(5))
+	role := "manager"
+	escalationPolicyOne := fmt.Sprintf("tf-%s", acctest.RandString(5))
+	escalationPolicyTwo := fmt.Sprintf("tf-%s", acctest.RandString(5))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckPagerDutyTeamMembershipDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeams(userOne, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPagerDutyTeamMembershipExists("pagerduty_team_membership.one"),
+				),
+			},
+			{
+				Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsUpdated(userOne, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPagerDutyTeamMembershipNoExists("pagerduty_team_membership.one"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckPagerDutyTeamMembershipDestroy(s *terraform.State) error {
 	client, _ := testAccProvider.Meta().(*Config).Client()
 	for _, r := range s.RootModule().Resources {
@@ -297,3 +326,115 @@ resource "pagerduty_escalation_policy" "foo" {
 }
 `, user, team, role, escalationPolicy)
 }
+
+func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeams(user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo string) string {
+	return fmt.Sprintf(`
+resource "pagerduty_user" "one" {
+  name = "%[1]v"
+  email = "%[1][email protected]"
+}
+
+resource "pagerduty_team" "one" {
+  name        = "%[2]v"
+  description = "team_one"
+}
+
+resource "pagerduty_team" "two" {
+  name        = "%[3]v"
+  description = "team_two"
+}
+
+resource "pagerduty_team_membership" "one" {
+  user_id = pagerduty_user.one.id
+  team_id = pagerduty_team.one.id
+  role    = "%[4]v"
+}
+
+resource "pagerduty_team_membership" "two" {
+  user_id = pagerduty_user.one.id
+  team_id = pagerduty_team.two.id
+  role    = "%[4]v"
+}
+
+resource "pagerduty_escalation_policy" "one" {
+  name      = "%s"
+  num_loops = 2
+  teams     = [pagerduty_team.one.id]
+
+  rule {
+    escalation_delay_in_minutes = 10
+    target {
+      type = "user_reference"
+      id   = pagerduty_user.one.id
+    }
+  }
+}
+
+resource "pagerduty_escalation_policy" "two" {
+  name      = "%s"
+  num_loops = 2
+  teams     = [pagerduty_team.two.id]
+
+  rule {
+    escalation_delay_in_minutes = 10
+    target {
+      type = "user_reference"
+      id   = pagerduty_user.one.id
+    }
+  }
+}
+`, user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo)
+}
+
+func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsUpdated(user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo string) string {
+	return fmt.Sprintf(`
+resource "pagerduty_user" "one" {
+  name = "%[1]v"
+  email = "%[1][email protected]"
+}
+
+resource "pagerduty_team" "one" {
+  name        = "%[2]v"
+  description = "team_one"
+}
+
+resource "pagerduty_team" "two" {
+  name        = "%[3]v"
+  description = "team_two"
+}
+
+resource "pagerduty_team_membership" "two" {
+  user_id = pagerduty_user.one.id
+  team_id = pagerduty_team.two.id
+  role    = "%[4]v"
+}
+
+resource "pagerduty_escalation_policy" "one" {
+  name      = "%s"
+  num_loops = 2
+  teams     = [pagerduty_team.one.id]
+
+  rule {
+    escalation_delay_in_minutes = 10
+    target {
+      type = "user_reference"
+      id   = pagerduty_user.one.id
+    }
+  }
+}
+
+resource "pagerduty_escalation_policy" "two" {
+  name      = "%s"
+  num_loops = 2
+  teams     = [pagerduty_team.two.id]
+
+  rule {
+    escalation_delay_in_minutes = 10
+    target {
+      type = "user_reference"
+      id   = pagerduty_user.one.id
+    }
+  }
+}
+`, user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo)
+}