[bug]: Fix commit strategies when updating repos

github/resource_github_repository.go

@@ -469,7 +469,7 @@ func calculateSecurityAndAnalysis(d *schema.ResourceData) *github.SecurityAndAna
 }
 
 func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository {
-	return &github.Repository{
+	repository := &github.Repository{
 		Name:                github.String(d.Get("name").(string)),
 		Description:         github.String(d.Get("description").(string)),
 		Homepage:            github.String(d.Get("homepage_url").(string)),
@@ -493,6 +493,26 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository {
 		AllowUpdateBranch:   github.Bool(d.Get("allow_update_branch").(bool)),
 		SecurityAndAnalysis: calculateSecurityAndAnalysis(d),
 	}
+
+	// only configure merge commit if we are in commit merge strategy
+	allowMergeCommit, ok := d.Get("allow_merge_commit").(bool)
+	if ok {
+		if allowMergeCommit {
+			repository.MergeCommitTitle = github.String(d.Get("merge_commit_title").(string))
+			repository.MergeCommitMessage = github.String(d.Get("merge_commit_message").(string))
+		}
+	}
+
+	// only configure squash commit if we are in squash merge strategy
+	allowSquashMerge, ok := d.Get("allow_squash_merge").(bool)
+	if ok {
+		if allowSquashMerge {
+			repository.SquashMergeCommitTitle = github.String(d.Get("squash_merge_commit_title").(string))
+			repository.SquashMergeCommitMessage = github.String(d.Get("squash_merge_commit_message").(string))
+		}
+	}
+
+	return repository
 }
 
 func resourceGithubRepositoryCreate(d *schema.ResourceData, meta interface{}) error {
@@ -524,24 +544,6 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta interface{}) er
 		}
 	}
 
-	// only configure merge commit if we are in commit merge strategy
-	allowMergeCommit, ok := d.Get("allow_merge_commit").(bool)
-	if ok {
-		if allowMergeCommit {
-			repoReq.MergeCommitTitle = github.String(d.Get("merge_commit_title").(string))
-			repoReq.MergeCommitMessage = github.String(d.Get("merge_commit_message").(string))
-		}
-	}
-
-	// only configure squash commit if we are in squash merge strategy
-	allowSquashMerge, ok := d.Get("allow_squash_merge").(bool)
-	if ok {
-		if allowSquashMerge {
-			repoReq.SquashMergeCommitTitle = github.String(d.Get("squash_merge_commit_title").(string))
-			repoReq.SquashMergeCommitMessage = github.String(d.Get("squash_merge_commit_message").(string))
-		}
-	}
-
 	repoReq.Private = github.Bool(isPrivate)
 
 	if template, ok := d.GetOk("template"); ok {

github/resource_github_repository_test.go

@@ -771,36 +771,67 @@ func TestAccGithubRepositories(t *testing.T) {
 
 	})
 
-	t.Run("modify merge commit strategy without error", func(t *testing.T) {
-		config := fmt.Sprintf(`
-			resource "github_repository" "test" {
-
-			  name                 = "tf-acc-test-modify-co-str-%[1]s"
-			  allow_merge_commit   = true
-			  merge_commit_title   = "PR_TITLE"
-			  merge_commit_message = "BLANK"
-			}
-		`, randomID)
+	t.Run("create and modify merge commit strategy without error", func(t *testing.T) {
+		mergeCommitTitle := "PR_TITLE"
+		mergeCommitMessage := "BLANK"
+		updatedMergeCommitTitle := "MERGE_MESSAGE"
+		updatedMergeCommitMessage := "PR_TITLE"
+
+		configs := map[string]string{
+			"before": fmt.Sprintf(`
+                		resource "github_repository" "test" {
+
+		                	name                 = "tf-acc-test-modify-co-str-%[1]s"
+		                  	allow_merge_commit   = true
+		                  	merge_commit_title   = "%s"
+		                  	merge_commit_message = "%s"
+		                }
+		        `, randomID, mergeCommitTitle, mergeCommitMessage),
+			"after": fmt.Sprintf(`
+		                resource "github_repository" "test" {
+		                  	name                 = "tf-acc-test-modify-co-str-%[1]s"
+		                  	allow_merge_commit   = true
+		                  	merge_commit_title   = "%s"
+		                  	merge_commit_message = "%s"
+		                }
+		        `, randomID, updatedMergeCommitTitle, updatedMergeCommitMessage),
+		}
 
-		check := resource.ComposeTestCheckFunc(
-			resource.TestCheckResourceAttr(
-				"github_repository.test", "merge_commit_title",
-				"PR_TITLE",
+		checks := map[string]resource.TestCheckFunc{
+			"before": resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "merge_commit_title",
+					mergeCommitTitle,
+				),
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "merge_commit_message",
+					mergeCommitMessage,
+				),
 			),
-			resource.TestCheckResourceAttr(
-				"github_repository.test", "merge_commit_message",
-				"BLANK",
+			"after": resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "merge_commit_title",
+					updatedMergeCommitTitle,
+				),
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "merge_commit_message",
+					updatedMergeCommitMessage,
+				),
 			),
-		)
+		}
 
 		testCase := func(t *testing.T, mode string) {
 			resource.Test(t, resource.TestCase{
 				PreCheck:  func() { skipUnlessMode(t, mode) },
 				Providers: testAccProviders,
 				Steps: []resource.TestStep{
 					{
-						Config: config,
-						Check:  check,
+						Config: configs["before"],
+						Check:  checks["before"],
+					},
+					{
+						Config: configs["after"],
+						Check:  checks["after"],
 					},
 				},
 			})
@@ -819,35 +850,66 @@ func TestAccGithubRepositories(t *testing.T) {
 		})
 	})
 
-	t.Run("modify squash merge strategy without error", func(t *testing.T) {
-		config := fmt.Sprintf(`
-			resource "github_repository" "test" {
-			  name                        = "tf-acc-test-modify-sq-str-%[1]s"
-			  allow_squash_merge          = true
-			  squash_merge_commit_title   = "PR_TITLE"
-			  squash_merge_commit_message = "BLANK"
-			}
-		`, randomID)
+	t.Run("create and modify squash merge commit strategy without error", func(t *testing.T) {
+		squashMergeCommitTitle := "PR_TITLE"
+		squashMergeCommitMessage := "PR_BODY"
+		updatedSquashMergeCommitTitle := "COMMIT_OR_PR_TITLE"
+		updatedSquashMergeCommitMessage := "COMMIT_MESSAGES"
+
+		configs := map[string]string{
+			"before": fmt.Sprintf(`
+	                	resource "github_repository" "test" {
+	                  		name                        = "tf-acc-test-modify-sq-str-%[1]s"
+	                  		allow_squash_merge          = true
+	                  		squash_merge_commit_title   = "%s"
+	                  		squash_merge_commit_message = "%s"
+	                	}
+	            	`, randomID, squashMergeCommitTitle, squashMergeCommitMessage),
+			"after": fmt.Sprintf(`
+	                	resource "github_repository" "test" {
+	                  		name                        = "tf-acc-test-modify-sq-str-%[1]s"
+	                  		allow_squash_merge          = true
+	                  		squash_merge_commit_title   = "%s"
+	                  		squash_merge_commit_message = "%s"
+	                	}
+	            	`, randomID, updatedSquashMergeCommitTitle, updatedSquashMergeCommitMessage),
+		}
 
-		check := resource.ComposeTestCheckFunc(
-			resource.TestCheckResourceAttr(
-				"github_repository.test", "squash_merge_commit_title",
-				"PR_TITLE",
+		checks := map[string]resource.TestCheckFunc{
+			"before": resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "squash_merge_commit_title",
+					squashMergeCommitTitle,
+				),
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "squash_merge_commit_message",
+					squashMergeCommitMessage,
+				),
 			),
-			resource.TestCheckResourceAttr(
-				"github_repository.test", "squash_merge_commit_message",
-				"BLANK",
+			"after": resource.ComposeTestCheckFunc(
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "squash_merge_commit_title",
+					updatedSquashMergeCommitTitle,
+				),
+				resource.TestCheckResourceAttr(
+					"github_repository.test", "squash_merge_commit_message",
+					updatedSquashMergeCommitMessage,
+				),
 			),
-		)
+		}
 
 		testCase := func(t *testing.T, mode string) {
 			resource.Test(t, resource.TestCase{
 				PreCheck:  func() { skipUnlessMode(t, mode) },
 				Providers: testAccProviders,
 				Steps: []resource.TestStep{
 					{
-						Config: config,
-						Check:  check,
+						Config: configs["before"],
+						Check:  checks["before"],
+					},
+					{
+						Config: configs["after"],
+						Check:  checks["after"],
 					},
 				},
 			})
@@ -865,6 +927,7 @@ func TestAccGithubRepositories(t *testing.T) {
 			testCase(t, organization)
 		})
 	})
+
 	t.Run("create a repository with go as primary_language", func(t *testing.T) {
 		config := fmt.Sprintf(`
 			resource "github_repository" "test" {

website/docs/r/repository.html.markdown

@@ -84,13 +84,13 @@ The following arguments are supported:
 
 * `allow_auto_merge` - (Optional) Set to `true` to allow auto-merging pull requests on the repository.
 
-* `squash_merge_commit_title` - (Optional) Can be `PR_TITLE` or `COMMIT_OR_PR_TITLE` for a default squash merge commit title.
+* `squash_merge_commit_title` - (Optional) Can be `PR_TITLE` or `COMMIT_OR_PR_TITLE` for a default squash merge commit title. Applicable only if `allow_squash_merge` is `true`.
 
-* `squash_merge_commit_message` - (Optional) Can be `PR_BODY`, `COMMIT_MESSAGES`, or `BLANK` for a default squash merge commit message.
+* `squash_merge_commit_message` - (Optional) Can be `PR_BODY`, `COMMIT_MESSAGES`, or `BLANK` for a default squash merge commit message. Applicable only if `allow_squash_merge` is `true`.
 
-* `merge_commit_title` - Can be `PR_TITLE` or `MERGE_MESSAGE` for a default merge commit title.
+* `merge_commit_title` - Can be `PR_TITLE` or `MERGE_MESSAGE` for a default merge commit title. Applicable only if `allow_merge_commit` is `true`.
 
-* `merge_commit_message` - Can be `PR_BODY`, `PR_TITLE`, or `BLANK` for a default merge commit message.
+* `merge_commit_message` - Can be `PR_BODY`, `PR_TITLE`, or `BLANK` for a default merge commit message. Applicable only if `allow_merge_commit` is `true`.
 
 * `delete_branch_on_merge` - (Optional) Automatically delete head branch after a pull request is merged. Defaults to `false`.