Merge pull request #453 from terraform-providers/ap_branch_protection…

…_dismissals only enable dismissal restrictions if users or teams provided
integrations · May 15, 2020 · 5ae8c3e · 5ae8c3e
2 parents 3171994 + caace39
commit 5ae8c3e
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 2 deletions.
diff --git a/github/resource_github_branch_protection.go b/github/resource_github_branch_protection.go
@@ -544,9 +544,13 @@ func expandRequiredPullRequestReviews(d *schema.ResourceData) (*github.PullReque
 			m := v.(map[string]interface{})
 
 			users := expandNestedSet(m, "dismissal_users")
-			drr.Users = &users
+			if len(users) > 0 {
+				drr.Users = &users
+			}
 			teams := expandNestedSet(m, "dismissal_teams")
-			drr.Teams = &teams
+			if len(teams) > 0 {
+				drr.Teams = &teams
+			}
 
 			rprr.DismissalRestrictionsRequest = drr
 			rprr.DismissStaleReviews = m["dismiss_stale_reviews"].(bool)

diff --git a/github/resource_github_branch_protection_test.go b/github/resource_github_branch_protection_test.go
@@ -218,6 +218,37 @@ func TestAccGithubBranchProtection_emptyItems(t *testing.T) {
 	})
 }
 
+func TestAccGithubBranchProtection_emptyDismissalRestrictions(t *testing.T) {
+	var protection github.Protection
+	rn := "github_branch_protection.master"
+	repoName := acctest.RandomWithPrefix("tf-acc-test-branch-prot-")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccGithubBranchProtectionDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccGithubBranchProtectionEmptyDismissalsConfig(repoName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGithubProtectedBranchExists("github_branch_protection.master", repoName+":master", &protection),
+					testAccCheckGithubBranchProtectionPullRequestReviews(&protection, true, []string{}, []string{}, true),
+					resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.#", "1"),
+					resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismiss_stale_reviews", "true"),
+					resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.require_code_owner_reviews", "true"),
+					resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_users.#", "0"),
+					resource.TestCheckResourceAttr(rn, "required_pull_request_reviews.0.dismissal_teams.#", "0"),
+				),
+			},
+			{
+				ResourceName:      rn,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func testAccCheckGithubProtectedBranchExists(n, id string, protection *github.Protection) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -307,6 +338,9 @@ func testAccCheckGithubBranchProtectionPullRequestReviews(protection *github.Pro
 
 		var users, teams []string
 		if reviews.DismissalRestrictions != nil {
+			if len(expectedUsers) == 0 && len(expectedTeams) == 0 {
+				return fmt.Errorf("Expected Dismissal Restrictions to be nil but was present")
+			}
 			for _, u := range reviews.GetDismissalRestrictions().Users {
 				users = append(users, u.GetLogin())
 			}
@@ -529,3 +563,25 @@ resource "github_branch_protection" "master" {
 }
 `, repoName, repoName, user)
 }
+
+func testAccGithubBranchProtectionEmptyDismissalsConfig(repoName string) string {
+	return fmt.Sprintf(`
+
+resource "github_repository" "test" {
+	name        = "%s"
+	description = "Terraform Acceptance Test %s"
+	auto_init   = true
+}
+
+resource "github_branch_protection" "master" {
+	repository     = "${github_repository.test.name}"
+	branch         = "master"
+	enforce_admins = true
+
+	required_pull_request_reviews {
+		dismiss_stale_reviews = true
+		require_code_owner_reviews = true
+	}
+}
+`, repoName, repoName)
+}