Merge pull request #81 from PostHog/kefapps/kef-175-normalize-posthog_insight-query_json-state-to-avoid-import

fix: normalize imported insight queries

Author: vdekrijger

internal/resource/hog_function.go

@@ -18,6 +18,13 @@ import (
 	"github.com/posthog/terraform-provider/internal/util"
 )
 
+// hogFunctionServerFields are fields the server generates on hog-function
+// responses and should be stripped to avoid spurious diffs.
+var hogFunctionServerFields = map[string]struct{}{
+	"bytecode": {},
+	"order":    {},
+}
+
 func NewHogFunction() resource.Resource {
 	return core.NewGenericResource[HogFunctionResourceTFModel, httpclient.HogFunctionRequest, httpclient.HogFunction](
 		HogFunctionOps{},
@@ -498,35 +505,6 @@ func normalizeJSONStripServerFields(apiData interface{}, userJSON string) (strin
 		return "", nil
 	}
 
-	cleaned := stripServerFields(apiData)
+	cleaned := util.StripFields(apiData, hogFunctionServerFields)
 	return normalizeJSONForState(cleaned, userJSON)
 }
-
-// stripServerFields recursively removes server-computed fields from a value.
-func stripServerFields(v interface{}) interface{} {
-	// serverComputedFields are fields that the server generates and should be stripped
-	// from API responses to avoid spurious diffs.
-	var serverComputedFields = map[string]struct{}{
-		"bytecode": {},
-		"order":    {},
-	}
-
-	switch val := v.(type) {
-	case map[string]interface{}:
-		cleaned := make(map[string]interface{})
-		for k, v := range val {
-			if _, isServerField := serverComputedFields[k]; !isServerField {
-				cleaned[k] = stripServerFields(v)
-			}
-		}
-		return cleaned
-	case []interface{}:
-		cleaned := make([]interface{}, len(val))
-		for i, item := range val {
-			cleaned[i] = stripServerFields(item)
-		}
-		return cleaned
-	default:
-		return v
-	}
-}

internal/resource/hog_function_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/posthog/terraform-provider/internal/httpclient"
+	"github.com/posthog/terraform-provider/internal/util"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -699,7 +700,7 @@ func TestStripServerFields(t *testing.T) {
 
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
-			result := stripServerFields(tt.input)
+			result := util.StripFields(tt.input, hogFunctionServerFields)
 			assert.Equal(t, tt.expected, result)
 		})
 	}

internal/resource/insight.go

@@ -19,6 +19,15 @@ import (
 	"github.com/posthog/terraform-provider/internal/util"
 )
 
+// insightQueryServerFields are query-internal fields that PostHog injects on
+// response and would otherwise produce drift after import.
+var insightQueryServerFields = map[string]struct{}{
+	"version":   {},
+	"result":    {},
+	"hogql":     {},
+	"is_cached": {},
+}
+
 func NewInsight() resource.Resource {
 	return core.NewGenericResource[InsightResourceTFModel, httpclient.InsightRequest, httpclient.Insight](
 		InsightOps{},
@@ -251,14 +260,21 @@ func normalizeQueryForState(apiQuery map[string]interface{}, userQueryJSON strin
 		return "", nil
 	}
 
+	cleanedQuery := util.StripFields(apiQuery, insightQueryServerFields)
+
+	userQueryJSON = strings.TrimSpace(userQueryJSON)
+	if userQueryJSON == "" {
+		return marshalJSON(cleanedQuery)
+	}
+
 	// Parse user's query to know which fields to keep
 	var userQuery map[string]interface{}
 	if err := json.Unmarshal([]byte(userQueryJSON), &userQuery); err != nil {
-		// If we can't parse user's query, fall back to returning API query as canonical JSON
-		return marshalJSON(apiQuery)
+		// If we can't parse user's query, fall back to returning a cleaned API query.
+		return marshalJSON(cleanedQuery)
 	}
 
-	filtered := filterToOnlyIncludeUserFields(userQuery, apiQuery)
+	filtered := filterToOnlyIncludeUserFields(userQuery, cleanedQuery)
 	return marshalJSON(filtered)
 }
 

internal/resource/insight_test.go

@@ -1,8 +1,11 @@
 package resource
 
 import (
+	"context"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/posthog/terraform-provider/internal/httpclient"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -192,11 +195,26 @@ func TestNormalizeQueryForState(t *testing.T) {
 		},
 		"falls back on invalid user JSON": {
 			apiQuery: map[string]interface{}{
-				"kind": "test",
+				"kind":    "test",
+				"version": float64(2),
 			},
 			userQueryJSON: `invalid json`,
 			expected:      `{"kind":"test"}`,
 		},
+		"strips server fields when user query is unavailable": {
+			apiQuery: map[string]interface{}{
+				"kind":      "DataVisualizationNode",
+				"result":    []interface{}{1, 2, 3},
+				"hogql":     "SELECT 1",
+				"is_cached": true,
+				"source": map[string]interface{}{
+					"kind":    "TrendsQuery",
+					"version": float64(2),
+				},
+			},
+			userQueryJSON: "",
+			expected:      `{"kind":"DataVisualizationNode","source":{"kind":"TrendsQuery"}}`,
+		},
 	}
 
 	for name, tt := range tests {
@@ -207,3 +225,28 @@ func TestNormalizeQueryForState(t *testing.T) {
 		})
 	}
 }
+
+func TestInsightMapResponseToModel_StripsServerQueryFieldsWithoutUserConfig(t *testing.T) {
+	ops := InsightOps{}
+	model := InsightResourceTFModel{
+		QueryJSON: types.StringNull(),
+	}
+
+	resp := httpclient.Insight{
+		ID: 123,
+		Query: map[string]interface{}{
+			"kind":      "DataVisualizationNode",
+			"result":    []interface{}{1, 2, 3},
+			"hogql":     "SELECT 1",
+			"is_cached": true,
+			"source": map[string]interface{}{
+				"kind":    "TrendsQuery",
+				"version": float64(2),
+			},
+		},
+	}
+
+	diags := ops.MapResponseToModel(context.Background(), resp, &model)
+	require.False(t, diags.HasError(), diags.Errors())
+	assert.Equal(t, `{"kind":"DataVisualizationNode","source":{"kind":"TrendsQuery"}}`, model.QueryJSON.ValueString())
+}

internal/util/json.go

@@ -0,0 +1,26 @@
+package util
+
+// StripFields recursively returns a copy of v with any map keys named in
+// denylist removed. v is expected to be a JSON-decoded value (map, slice,
+// or scalar); other types are returned as-is.
+func StripFields(v interface{}, denylist map[string]struct{}) interface{} {
+	switch val := v.(type) {
+	case map[string]interface{}:
+		cleaned := make(map[string]interface{}, len(val))
+		for key, value := range val {
+			if _, isStripped := denylist[key]; isStripped {
+				continue
+			}
+			cleaned[key] = StripFields(value, denylist)
+		}
+		return cleaned
+	case []interface{}:
+		cleaned := make([]interface{}, len(val))
+		for i, item := range val {
+			cleaned[i] = StripFields(item, denylist)
+		}
+		return cleaned
+	default:
+		return val
+	}
+}

testacc/insight_test.go

@@ -300,7 +300,7 @@ func TestInsight_Import(t *testing.T) {
 				ResourceName:            "posthog_insight.test",
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"query_json", "create_in_folder"},
+				ImportStateVerifyIgnore: []string{"create_in_folder"},
 			},
 		},
 	})