From 79c359ad0d425380975dbe282a091ce96370452e Mon Sep 17 00:00:00 2001
From: Jacob Bednarz <jacob.bednarz@gmail.com>
Date: Fri, 23 Aug 2019 02:20:03 +1000
Subject: [PATCH] page_rule: Remove `HasChange` for `edge_cache_ttl` (#453)

Prior to swapping to PUT requests for updating page rules, we used to
perform manual diff checks that would determine which actions we would
add to the page rule before sending it. This is no longer now as we
swapped to using a PUT request which replaces the whole rule and we are
able to send the whole page rule as-is without doing the work ourselves.

This popped up as it's caused a bug whereby updating a page rule that
includes a `edge_cache_ttl` (but not change it) results in the
`edge_cache_ttl` being dropped completely.

`TestAccCloudflarePageRuleEdgeCacheTTLNotClobbered` is the regression
test case that demonstrates the behaviour.
---
 cloudflare/resource_cloudflare_page_rule.go   |  4 +-
 .../resource_cloudflare_page_rule_test.go     | 53 +++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/cloudflare/resource_cloudflare_page_rule.go b/cloudflare/resource_cloudflare_page_rule.go
index 1a43c18f52..42881f0418 100644
--- a/cloudflare/resource_cloudflare_page_rule.go
+++ b/cloudflare/resource_cloudflare_page_rule.go
@@ -605,8 +605,6 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema
 
 	pageRuleAction.ID = id
 
-	changed := d.HasChange(fmt.Sprintf("actions.0.%s", id))
-
 	if strValue, ok := value.(string); ok {
 		if id == "browser_cache_ttl" {
 			intValue, err := strconv.Atoi(strValue)
@@ -633,7 +631,7 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema
 			}
 		}
 	} else if intValue, ok := value.(int); ok {
-		if id == "edge_cache_ttl" && intValue > 0 && changed {
+		if id == "edge_cache_ttl" && intValue > 0 {
 			pageRuleAction.Value = intValue
 		} else {
 			pageRuleAction.Value = nil
diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go
index 7f9f3a2f81..7338064658 100644
--- a/cloudflare/resource_cloudflare_page_rule_test.go
+++ b/cloudflare/resource_cloudflare_page_rule_test.go
@@ -348,6 +348,34 @@ func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders
 	})
 }
 
+func TestAccCloudflarePageRuleEdgeCacheTTLNotClobbered(t *testing.T) {
+	var before, after cloudflare.PageRule
+	zone := os.Getenv("CLOUDFLARE_DOMAIN")
+	target := fmt.Sprintf("test-edge-cache-ttl-not-clobbered.%s", zone)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckCloudflarePageRuleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCheckCloudflarePageRuleConfigWithEdgeCacheTtl(zone, target),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &before),
+					resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.edge_cache_ttl", "10"),
+				),
+			},
+			{
+				Config: testAccCheckCloudflarePageRuleConfigWithEdgeCacheTtlAndAlwaysOnline(zone, target),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &after),
+					resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.edge_cache_ttl", "10"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckCloudflarePageRuleRecreated(before, after *cloudflare.PageRule) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		if before.ID == after.ID {
@@ -603,6 +631,31 @@ resource "cloudflare_page_rule" "test" {
 }`, zone, target)
 }
 
+func testAccCheckCloudflarePageRuleConfigWithEdgeCacheTtl(zone, target string) string {
+	return fmt.Sprintf(`
+resource "cloudflare_page_rule" "test" {
+	zone = "%s"
+	target = "%s"
+	actions {
+		always_online = "on"
+		ssl = "flexible"
+		edge_cache_ttl = 10
+	}
+}`, zone, target)
+}
+
+func testAccCheckCloudflarePageRuleConfigWithEdgeCacheTtlAndAlwaysOnline(zone, target string) string {
+	return fmt.Sprintf(`
+resource "cloudflare_page_rule" "test" {
+	zone = "%s"
+	target = "%s"
+	actions {
+		always_online = "on"
+		edge_cache_ttl = 10
+	}
+}`, zone, target)
+}
+
 func buildPageRuleConfig(resourceName string, actions string) string {
 	zone := os.Getenv("CLOUDFLARE_DOMAIN")
 	target := fmt.Sprintf("terraform-test.%s", zone)