From eb675502468dd8e9378d3aef7c046d36bd8ecdfa Mon Sep 17 00:00:00 2001
From: Sean Marciniak <smarciniak@splunk.com>
Date: Thu, 24 Oct 2024 14:17:48 +1030
Subject: [PATCH 1/4] Adding current state of detector improvements

---
 internal/common/notification_list.go          |   1 +
 internal/definition/detector/resource.go      |  45 +++++
 .../detector/resource_acceptance_test.go      | 174 ++++++++++++++++++
 internal/definition/detector/resource_test.go |  20 +-
 internal/definition/detector/schema.go        |  10 +-
 .../detector/testdata/advanced_00.tf          |  45 +++++
 .../detector/testdata/advanced_01.tf          |  42 +++++
 .../detector/testdata/advanced_02.tf          |  33 ++++
 .../definition/detector/testdata/minimal.tf   |  16 ++
 internal/definition/rule/encoding.go          |   8 +-
 internal/definition/team/schema.go            |   3 +-
 internal/tfextension/encoding.go              |   4 +-
 internal/tfextension/values.go                |  23 +++
 internal/tfextension/values_test.go           |   4 +
 internal/tftest/resource_test.go              |  28 +--
 internal/tftest/state.go                      |  18 ++
 internal/tftest/state_test.go                 |   1 +
 17 files changed, 436 insertions(+), 39 deletions(-)
 create mode 100644 internal/definition/detector/resource_acceptance_test.go
 create mode 100644 internal/definition/detector/testdata/advanced_00.tf
 create mode 100644 internal/definition/detector/testdata/advanced_01.tf
 create mode 100644 internal/definition/detector/testdata/advanced_02.tf
 create mode 100644 internal/definition/detector/testdata/minimal.tf
 create mode 100644 internal/tfextension/values.go
 create mode 100644 internal/tfextension/values_test.go
 create mode 100644 internal/tftest/state.go
 create mode 100644 internal/tftest/state_test.go

diff --git a/internal/common/notification_list.go b/internal/common/notification_list.go
index 42b89a88..48718d3b 100644
--- a/internal/common/notification_list.go
+++ b/internal/common/notification_list.go
@@ -11,6 +11,7 @@ func NewNotificationList(items []any) ([]*notification.Notification, error) {
 	if len(items) == 0 {
 		return nil, nil
 	}
+
 	values := make([]*notification.Notification, len(items))
 	for i, v := range items {
 		n, err := NewNotificationFromString(v.(string))
diff --git a/internal/definition/detector/resource.go b/internal/definition/detector/resource.go
index c7b76911..da47c872 100644
--- a/internal/definition/detector/resource.go
+++ b/internal/definition/detector/resource.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/signalfx/signalfx-go/detector"
 
@@ -24,6 +25,7 @@ const (
 
 func NewResource() *schema.Resource {
 	return &schema.Resource{
+		SchemaVersion: 1,
 		SchemaFunc:    newSchema,
 		CreateContext: resourceCreate,
 		ReadContext:   resourceRead,
@@ -35,6 +37,7 @@ func NewResource() *schema.Resource {
 		StateUpgraders: []schema.StateUpgrader{
 			{Type: v0state().CoreConfigSchema().ImpliedType(), Upgrade: v0stateMigration, Version: 0},
 		},
+		CustomizeDiff: customdiff.If(resourceValidateCond, resourceValidateFunc),
 	}
 }
 
@@ -158,3 +161,45 @@ func resourceDelete(ctx context.Context, data *schema.ResourceData, meta any) di
 	err = common.HandleError(ctx, client.DeleteDetector(ctx, data.Id()), data)
 	return tfext.AsErrorDiagnostics(err)
 }
+
+func resourceValidateCond(ctx context.Context, diff *schema.ResourceDiff, _ any) (validate bool) {
+	tflog.Debug(ctx, "Checking if program text or rules needed to be updated")
+	if _, ok := diff.GetOkExists("rule"); ok {
+		validate = true
+	}
+	if _, ok := diff.GetOkExists("program_text"); ok {
+		validate = true
+	}
+	return validate
+}
+
+func resourceValidateFunc(ctx context.Context, diff *schema.ResourceDiff, meta any) error {
+	var rules []*detector.Rule
+	for _, v := range diff.Get("rule").(*schema.Set).List() {
+		data := v.(map[string]any)
+		rules = append(rules, &detector.Rule{
+			Description:          data["description"].(string),
+			DetectLabel:          data["detect_label"].(string),
+			Disabled:             data["disabled"].(bool),
+			Severity:             detector.Severity(data["severity"].(string)),
+			ParameterizedBody:    data["parameterized_body"].(string),
+			ParameterizedSubject: data["parameterized_subject"].(string),
+			RunbookUrl:           data["runbook_url"].(string),
+			Tip:                  data["tip"].(string),
+		})
+	}
+
+	client, err := pmeta.LoadClient(ctx, meta)
+	if err != nil {
+		return err
+	}
+
+	tflog.Debug(ctx, "Sending detector payload for validation", tfext.NewLogFields().JSON("content", rules))
+	return client.ValidateDetector(ctx, &detector.ValidateDetectorRequestModel{
+		Name:             diff.Get("name").(string),
+		ProgramText:      diff.Get("program_text").(string),
+		Rules:            rules,
+		DetectorOrigin:   diff.Get("detector_origin").(string),
+		ParentDetectorId: diff.Get("parent_detector_id").(string),
+	})
+}
diff --git a/internal/definition/detector/resource_acceptance_test.go b/internal/definition/detector/resource_acceptance_test.go
new file mode 100644
index 00000000..c0510c8b
--- /dev/null
+++ b/internal/definition/detector/resource_acceptance_test.go
@@ -0,0 +1,174 @@
+// Copyright Splunk, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package detector_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+
+	"github.com/splunk-terraform/terraform-provider-signalfx/internal/definition/detector"
+	"github.com/splunk-terraform/terraform-provider-signalfx/internal/definition/team"
+	"github.com/splunk-terraform/terraform-provider-signalfx/internal/tftest"
+)
+
+func TestAcceptance(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		steps []resource.TestStep
+	}{
+		{
+			name: "minimal detector",
+			steps: []resource.TestStep{
+				{
+					Config: tftest.LoadConfig("testdata/minimal.tf"),
+					Check: resource.ComposeAggregateTestCheckFunc(
+						resource.TestCheckResourceAttr("signalfx_detector.minimal", "name", "my minimal detector"),
+						resource.TestCheckResourceAttr("signalfx_detector.minimal", "program_text", "detect(when(const(1) > 1)).publish('HCF')\n"),
+					),
+					ExpectNonEmptyPlan: true,
+				},
+			},
+		},
+		{
+			name: "advanced detector",
+			steps: []resource.TestStep{
+				{
+					Config: tftest.LoadConfig("testdata/advanced_00.tf"),
+					Check: resource.ComposeAggregateTestCheckFunc(
+						tftest.DelayStateCheck(60*time.Second),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "example detector"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "A detector made from terraform"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.#", "2"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.0", "tag-1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.1", "tag-2"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "teams.#", "1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "30"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "15"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.%", "2"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 30m", "1000"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 5m", "1000"),
+
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "2"),
+
+						// Rule #1
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.description", "maximum > 60 for 5m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.detect_label", "Processing old messages 5m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.disabled", "false"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.notifications.#", "1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_body", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_subject", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.runbook_url", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.severity", "Warning"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.tip", ""),
+
+						// Rule #2
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.description", "maximum > 60 for 30m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.detect_label", "Processing old messages 30m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.disabled", "false"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.#", "1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_body", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_subject", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.runbook_url", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.severity", "Critical"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.tip", ""),
+					),
+					ExpectNonEmptyPlan: false,
+					Destroy:            false,
+				},
+				{
+					Config: tftest.LoadConfig("testdata/advanced_01.tf"),
+					Check: resource.ComposeAggregateTestCheckFunc(
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "name", "max average delay UPDATED"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "description", "your application is slowER"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "timezone", "Europe/Paris"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.#", "3"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.0", "tag-1"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.1", "tag-2"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.2", "tag-3"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "teams.#", "0"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "max_delay", "60"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "min_delay", "15"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay",
+							"time_range", "3600"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_data_markers", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_event_lines", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "disable_sampling", "true"),
+
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "label_resolutions.%", "2"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "label_resolutions.Processing old messages 30m", "1000"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "label_resolutions.Processing old messages 5m", "1000"),
+
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.#", "2"),
+
+						// Rule #1
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.description", "NEW maximum > 60 for 5m"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.parameterized_body", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.parameterized_subject", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.severity", "Warning"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.runbook_url", "https://www.example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.tip", "reboot it"),
+
+						// Rule #2
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.description", "NEW maximum > 60 for 30m"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.detect_label", "Processing old messages 30m"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.disabled", "false"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.notifications.#", "1"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.parameterized_body", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.parameterized_subject", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.runbook_url", "https://www.example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.severity", "Critical"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.tip", ""),
+					),
+				},
+				{
+					Config: tftest.LoadConfig("testdata/advanced_02.tf"),
+					Check: resource.ComposeAggregateTestCheckFunc(
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "name", "max average delay UPDATED"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "description", "your application is slowER"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "timezone", "Europe/Paris"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.#", "0"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "teams.#", "0"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "max_delay", "60"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "min_delay", "30"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay",
+							"time_range", "3600"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\n"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_data_markers", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_event_lines", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "disable_sampling", "true"),
+
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.#", "1"),
+
+						// Rule #1
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.description", "NEW maximum > 60 for 5m"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.severity", "Warning"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.runbook_url", "https://www.example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.tip", "reboot it"),
+					),
+				},
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			tftest.NewAcceptanceHandler(
+				tftest.WithAcceptanceResources(map[string]*schema.Resource{
+					detector.ResourceName: detector.NewResource(),
+					team.ResourceName:     team.NewResource(),
+				}),
+			).
+				Test(t, tc.steps)
+		})
+	}
+}
diff --git a/internal/definition/detector/resource_test.go b/internal/definition/detector/resource_test.go
index bc2539bd..b307734d 100644
--- a/internal/definition/detector/resource_test.go
+++ b/internal/definition/detector/resource_test.go
@@ -97,8 +97,8 @@ func TestResourceCreate(t *testing.T) {
 				Description:       "An example detector response",
 				AuthorizedWriters: &detector.AuthorizedWriters{},
 				TimeZone:          "Australia/Sydney",
-				MaxDelay:          common.AsPointer[int32](100),
-				MinDelay:          common.AsPointer[int32](100),
+				MaxDelay:          common.AsPointer[int32](1300),
+				MinDelay:          common.AsPointer[int32](1400),
 				ProgramText:       `detect(when(data('*').count() < 1)).publish('no data')`,
 				OverMTSLimit:      false,
 				Rules: []*detector.Rule{
@@ -120,8 +120,8 @@ func TestResourceCreate(t *testing.T) {
 				Description:       "An example detector response",
 				AuthorizedWriters: &detector.AuthorizedWriters{},
 				TimeZone:          "Australia/Sydney",
-				MaxDelay:          common.AsPointer[int32](100000000),
-				MinDelay:          common.AsPointer[int32](100000000),
+				MaxDelay:          common.AsPointer[int32](1000),
+				MinDelay:          common.AsPointer[int32](1000),
 				ProgramText:       `detect(when(data('*').count() < 1)).publish('no data')`,
 				OverMTSLimit:      false,
 				Rules: []*detector.Rule{
@@ -195,8 +195,8 @@ func TestResourceRead(t *testing.T) {
 						Name:        "test detector",
 						Description: "An example detector response",
 						TimeZone:    "Australia/Sydney",
-						MaxDelay:    common.AsPointer[int32](100),
-						MinDelay:    common.AsPointer[int32](100),
+						MaxDelay:    common.AsPointer[int32](1000),
+						MinDelay:    common.AsPointer[int32](1000),
 						ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
 						Rules: []*detector.Rule{
 							{
@@ -221,8 +221,8 @@ func TestResourceRead(t *testing.T) {
 				Description:       "An example detector response",
 				AuthorizedWriters: &detector.AuthorizedWriters{},
 				TimeZone:          "Australia/Sydney",
-				MaxDelay:          common.AsPointer[int32](100000),
-				MinDelay:          common.AsPointer[int32](100000),
+				MaxDelay:          common.AsPointer[int32](1000),
+				MinDelay:          common.AsPointer[int32](1000),
 				ProgramText:       `detect(when(data('*').count() < 1)).publish('no data')`,
 				Rules: []*detector.Rule{
 					{
@@ -280,8 +280,8 @@ func TestResourceRead(t *testing.T) {
 				Description:       "An example detector response",
 				AuthorizedWriters: &detector.AuthorizedWriters{},
 				TimeZone:          "Australia/Sydney",
-				MaxDelay:          common.AsPointer[int32](100000000),
-				MinDelay:          common.AsPointer[int32](100000000),
+				MaxDelay:          common.AsPointer[int32](100),
+				MinDelay:          common.AsPointer[int32](100),
 				ProgramText:       `detect(when(data('*').count() < 1)).publish('no data')`,
 				OverMTSLimit:      true,
 				Rules: []*detector.Rule{
diff --git a/internal/definition/detector/schema.go b/internal/definition/detector/schema.go
index b76fe5dd..a5364a38 100644
--- a/internal/definition/detector/schema.go
+++ b/internal/definition/detector/schema.go
@@ -194,7 +194,7 @@ func newSchema() map[string]*schema.Schema {
 	}
 }
 
-func decodeTerraform(rd *schema.ResourceData) (*detector.Detector, error) {
+func decodeTerraform(rd tfext.Values) (*detector.Detector, error) {
 	d := &detector.Detector{
 		Id:                rd.Id(),
 		Name:              rd.Get("name").(string),
@@ -207,7 +207,7 @@ func decodeTerraform(rd *schema.ResourceData) (*detector.Detector, error) {
 		//nolint:gosec // Overflow is not possible from config
 		MinDelay: common.AsPointer(int32(rd.Get("min_delay").(int)) * 1000),
 		//nolint:gosec // Overflow is not possible from config
-		MaxDelay: common.AsPointer(int32(rd.Get("min_delay").(int)) * 1000),
+		MaxDelay: common.AsPointer(int32(rd.Get("max_delay").(int)) * 1000),
 		VisualizationOptions: &detector.Visualization{
 			DisableSampling: rd.Get("disable_sampling").(bool),
 			ShowDataMarkers: rd.Get("show_data_markers").(bool),
@@ -285,11 +285,13 @@ func encodeTerraform(dt *detector.Detector, rd *schema.ResourceData) error {
 		rd.Set("tags", dt.Tags),
 		rd.Set("label_resolutions", dt.LabelResolutions),
 	)
+	// We divide by 1000 because the API uses millis, but this provider uses
+	// seconds
 	if dt.MinDelay != nil {
-		errs = multierr.Append(errs, rd.Set("min_delay", *dt.MinDelay))
+		errs = multierr.Append(errs, rd.Set("min_delay", *dt.MinDelay/1000))
 	}
 	if dt.MaxDelay != nil {
-		errs = multierr.Append(errs, rd.Set("max_delay", *dt.MaxDelay))
+		errs = multierr.Append(errs, rd.Set("max_delay", *dt.MaxDelay/1000))
 	}
 	if auth := dt.AuthorizedWriters; auth != nil {
 		errs = multierr.Append(errs, rd.Set("authorized_writer_teams", tfext.NewSchemaSet(schema.HashString, auth.Teams)))
diff --git a/internal/definition/detector/testdata/advanced_00.tf b/internal/definition/detector/testdata/advanced_00.tf
new file mode 100644
index 00000000..774f4934
--- /dev/null
+++ b/internal/definition/detector/testdata/advanced_00.tf
@@ -0,0 +1,45 @@
+provider "signalfx" {}
+
+resource "signalfx_team" "my_detector_team" {
+  name        = "example team"
+  description = "A team made from terraform"
+
+  notifications_default = ["Email,test@example.com"]
+}
+
+resource "signalfx_detector" "my_detector" {
+  name            = "example detector"
+  description     = "A detector made from terraform"
+  max_delay       = 30
+  min_delay       = 15
+  tags            = ["tag-1", "tag-2"]
+  teams           = [signalfx_team.my_detector_team.id]
+  timezone        = "Europe/Paris"
+  detector_origin = "Standard"
+
+  program_text = <<-EOF
+    signal = data('app.delay').max().publish('app delay')
+    detect(when(signal > 60, '5m')).publish('Processing old messages 5m')
+    detect(when(signal > 60, '30m')).publish('Processing old messages 30m')
+  EOF
+
+  rule {
+    description   = "maximum > 60 for 5m"
+    severity      = "Warning"
+    detect_label  = "Processing old messages 5m"
+    notifications = ["Email,foo-alerts@example.com"]
+  }
+
+  rule {
+    description   = "maximum > 60 for 30m"
+    severity      = "Critical"
+    detect_label  = "Processing old messages 30m"
+    notifications = ["Email,foo-alerts@example.com"]
+  }
+
+  viz_options {
+    label      = "app delay"
+    color      = "orange"
+    value_unit = "Second"
+  }
+}
diff --git a/internal/definition/detector/testdata/advanced_01.tf b/internal/definition/detector/testdata/advanced_01.tf
new file mode 100644
index 00000000..1060c4f0
--- /dev/null
+++ b/internal/definition/detector/testdata/advanced_01.tf
@@ -0,0 +1,42 @@
+resource "signalfx_detector" "my_detector" {
+  name        = "max average delay UPDATED"
+  description = "your application is slowER"
+  max_delay   = 60
+  min_delay   = 30
+  timezone    = "Europe/Paris"
+
+  show_data_markers = true
+  show_event_lines  = true
+  disable_sampling  = true
+  time_range        = 3600
+  tags              = ["tag-1", "tag-2", "tag-3"]
+
+  program_text = <<-EOF
+    signal = data('app.delay2').max().publish('app delay')
+    detect(when(signal > 60, '5m')).publish('Processing old messages 5m')
+    detect(when(signal > 60, '30m')).publish('Processing old messages 30m')
+  EOF
+
+  rule {
+    description   = "NEW maximum > 60 for 5m"
+    severity      = "Warning"
+    detect_label  = "Processing old messages 5m"
+    notifications = ["Email,foo-alerts@example.com"]
+    runbook_url   = "https://www.example.com"
+    tip           = "reboot it"
+  }
+
+  rule {
+    description   = "NEW maximum > 60 for 30m"
+    severity      = "Critical"
+    detect_label  = "Processing old messages 30m"
+    notifications = ["Email,foo-alerts@example.com"]
+    runbook_url   = "https://www.example.com"
+  }
+
+  viz_options {
+    label      = "app delay"
+    color      = "orange"
+    value_unit = "Second"
+  }
+}
diff --git a/internal/definition/detector/testdata/advanced_02.tf b/internal/definition/detector/testdata/advanced_02.tf
new file mode 100644
index 00000000..6f1b78c2
--- /dev/null
+++ b/internal/definition/detector/testdata/advanced_02.tf
@@ -0,0 +1,33 @@
+resource "signalfx_detector" "application_delay" {
+  name        = "max average delay UPDATED"
+  description = "your application is slowER"
+  max_delay   = 60
+  min_delay   = 30
+  timezone    = "Europe/Paris"
+
+  show_data_markers = true
+  show_event_lines  = true
+  disable_sampling  = true
+  time_range        = 3600
+
+  program_text = <<-EOF
+    signal = data('app.delay2').max().publish('app delay')
+    detect(when(signal > 60, '5m')).publish('Processing old messages 5m')
+  EOF
+
+  rule {
+    description   = "NEW maximum > 60 for 5m"
+    severity      = "Warning"
+    detect_label  = "Processing old messages 5m"
+    notifications = ["Email,foo-alerts@example.com"]
+    runbook_url   = "https://www.example.com"
+    tip           = "reboot it"
+  }
+
+  viz_options {
+    label      = "app delay"
+    color      = "orange"
+    value_unit = "Second"
+  }
+
+}
diff --git a/internal/definition/detector/testdata/minimal.tf b/internal/definition/detector/testdata/minimal.tf
new file mode 100644
index 00000000..666db8ed
--- /dev/null
+++ b/internal/definition/detector/testdata/minimal.tf
@@ -0,0 +1,16 @@
+provider "signalfx" {}
+
+resource "signalfx_detector" "minimal" {
+  name = "my minimal detector"
+
+  program_text = <<-EOF
+  detect(when(const(1) > 1)).publish('HCF')
+  EOF
+
+  rule {
+    description   = "example detector"
+    severity      = "Warning"
+    detect_label  = "HCF"
+    notifications = ["Email,test@example.com"]
+  }
+}
diff --git a/internal/definition/rule/encoding.go b/internal/definition/rule/encoding.go
index a2e7a570..630fb1c2 100644
--- a/internal/definition/rule/encoding.go
+++ b/internal/definition/rule/encoding.go
@@ -11,9 +11,10 @@ import (
 	"github.com/signalfx/signalfx-go/detector"
 
 	"github.com/splunk-terraform/terraform-provider-signalfx/internal/common"
+	tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension"
 )
 
-func DecodeTerraform(rd *schema.ResourceData) ([]*detector.Rule, error) {
+func DecodeTerraform(rd tfext.Values) ([]*detector.Rule, error) {
 	set, ok := rd.Get("rule").(*schema.Set)
 	if !ok {
 		return nil, errors.New("no field defined for rule")
@@ -44,6 +45,10 @@ func DecodeTerraform(rd *schema.ResourceData) ([]*detector.Rule, error) {
 }
 
 func EncodeTerraform(rules []*detector.Rule, rd *schema.ResourceData) error {
+	if len(rules) == 0 {
+		return nil
+	}
+
 	items := make([]map[string]any, 0, len(rules))
 	for _, r := range rules {
 		notifys, err := common.NewNotificationStringList(r.Notifications)
@@ -62,5 +67,6 @@ func EncodeTerraform(rules []*detector.Rule, rd *schema.ResourceData) error {
 			"tip":                   r.Tip,
 		})
 	}
+
 	return rd.Set("rule", items)
 }
diff --git a/internal/definition/team/schema.go b/internal/definition/team/schema.go
index f0dd4c38..7efdd024 100644
--- a/internal/definition/team/schema.go
+++ b/internal/definition/team/schema.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/splunk-terraform/terraform-provider-signalfx/internal/check"
 	"github.com/splunk-terraform/terraform-provider-signalfx/internal/common"
+	tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension"
 )
 
 func newSchema() map[string]*schema.Schema {
@@ -92,7 +93,7 @@ func newSchema() map[string]*schema.Schema {
 	}
 }
 
-func decodeTerraform(rd *schema.ResourceData) (*team.Team, error) {
+func decodeTerraform(rd tfext.Values) (*team.Team, error) {
 	t := &team.Team{
 		Id:          rd.Id(),
 		Name:        rd.Get("name").(string),
diff --git a/internal/tfextension/encoding.go b/internal/tfextension/encoding.go
index b83666eb..a5c83855 100644
--- a/internal/tfextension/encoding.go
+++ b/internal/tfextension/encoding.go
@@ -12,7 +12,7 @@ type (
 	// the terraform state data into the expected API type to be used.
 	//
 	// This is to be used when interfacing with other packages.
-	DecodeTerraformFunc[T any] func(rd *schema.ResourceData) (*T, error)
+	DecodeTerraformFunc[T any] func(rd Values) (*T, error)
 
 	// EncodeTerraformFunc is used to write the API response data into
 	// the terraform state.
@@ -21,7 +21,7 @@ type (
 	EncodeTerraformFunc[T any] func(t *T, rd *schema.ResourceData) error
 )
 
-func NopDecodeTerraform[T any](*schema.ResourceData) (*T, error) {
+func NopDecodeTerraform[T any](Values) (*T, error) {
 	return nil, nil
 }
 
diff --git a/internal/tfextension/values.go b/internal/tfextension/values.go
new file mode 100644
index 00000000..026df022
--- /dev/null
+++ b/internal/tfextension/values.go
@@ -0,0 +1,23 @@
+// Copyright Splunk, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package tfext
+
+import "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+
+// Values is a unified resource data type
+// to allow for high reusable methods in different scenarios.
+//
+// For method information, please refer to the defined types
+// below and check their API interface.
+type Values interface {
+	Id() string
+	Get(string) any
+	GetOk(string) (any, bool)
+	HasChanges(values ...string) bool
+}
+
+var (
+	_ Values = (*schema.ResourceData)(nil)
+	_ Values = (*schema.ResourceDiff)(nil)
+)
diff --git a/internal/tfextension/values_test.go b/internal/tfextension/values_test.go
new file mode 100644
index 00000000..4ba7f309
--- /dev/null
+++ b/internal/tfextension/values_test.go
@@ -0,0 +1,4 @@
+// Copyright Splunk, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package tfext
diff --git a/internal/tftest/resource_test.go b/internal/tftest/resource_test.go
index 77ce2571..0b8db334 100644
--- a/internal/tftest/resource_test.go
+++ b/internal/tftest/resource_test.go
@@ -95,9 +95,7 @@ func TestResourceOperationTestCaseCreate(t *testing.T) {
 			Encoder: func(*any, *schema.ResourceData) error {
 				return nil
 			},
-			Decoder: func(*schema.ResourceData) (*any, error) {
-				return nil, nil
-			},
+			Decoder: tfext.NopDecodeTerraform[any],
 			Meta: func(testing.TB) any {
 				return nil
 			},
@@ -186,12 +184,8 @@ func TestResourceOperationTestCaseRead(t *testing.T) {
 		tcase := &ResourceOperationTestCase[any]{
 			Name:     tc.name,
 			Resource: tc.resource,
-			Encoder: func(*any, *schema.ResourceData) error {
-				return nil
-			},
-			Decoder: func(*schema.ResourceData) (*any, error) {
-				return nil, nil
-			},
+			Encoder:  tfext.NopEncodeTerraform[any],
+			Decoder:  tfext.NopDecodeTerraform[any],
 			Meta: func(testing.TB) any {
 				return nil
 			},
@@ -280,12 +274,8 @@ func TestResourceOperationTestCaseUpdate(t *testing.T) {
 		tcase := &ResourceOperationTestCase[any]{
 			Name:     tc.name,
 			Resource: tc.resource,
-			Encoder: func(*any, *schema.ResourceData) error {
-				return nil
-			},
-			Decoder: func(*schema.ResourceData) (*any, error) {
-				return nil, nil
-			},
+			Encoder:  tfext.NopEncodeTerraform[any],
+			Decoder:  tfext.NopDecodeTerraform[any],
 			Meta: func(testing.TB) any {
 				return nil
 			},
@@ -374,12 +364,8 @@ func TestResourceOperationTestCaseDelete(t *testing.T) {
 		tcase := &ResourceOperationTestCase[any]{
 			Name:     tc.name,
 			Resource: tc.resource,
-			Encoder: func(*any, *schema.ResourceData) error {
-				return nil
-			},
-			Decoder: func(*schema.ResourceData) (*any, error) {
-				return nil, nil
-			},
+			Encoder:  tfext.NopEncodeTerraform[any],
+			Decoder:  tfext.NopDecodeTerraform[any],
 			Meta: func(testing.TB) any {
 				return nil
 			},
diff --git a/internal/tftest/state.go b/internal/tftest/state.go
new file mode 100644
index 00000000..167ce4ff
--- /dev/null
+++ b/internal/tftest/state.go
@@ -0,0 +1,18 @@
+package tftest
+
+import (
+	"time"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+)
+
+// DelayStateCheck should be used in times when the services
+// need some additional processing time to ensure that all the expected
+// values are populated.
+func DelayStateCheck(delay time.Duration) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		<-time.After(delay)
+		return nil
+	}
+}
diff --git a/internal/tftest/state_test.go b/internal/tftest/state_test.go
new file mode 100644
index 00000000..639d1257
--- /dev/null
+++ b/internal/tftest/state_test.go
@@ -0,0 +1 @@
+package tftest

From 3a2903a34982e1a17fc44e3a585760777ad9ad6b Mon Sep 17 00:00:00 2001
From: Sean Marciniak <smarciniak@splunk.com>
Date: Thu, 24 Oct 2024 20:39:42 +1030
Subject: [PATCH 2/4] Fixing up coverage

---
 internal/definition/detector/resource.go      |  13 +-
 .../detector/resource_acceptance_test.go      | 117 +++++++++---------
 internal/definition/detector/resource_test.go |  28 +++++
 internal/definition/detector/schema.go        |   2 +-
 .../detector/testdata/advanced_02.tf          |   2 +-
 5 files changed, 98 insertions(+), 64 deletions(-)

diff --git a/internal/definition/detector/resource.go b/internal/definition/detector/resource.go
index da47c872..5a4c89f9 100644
--- a/internal/definition/detector/resource.go
+++ b/internal/definition/detector/resource.go
@@ -79,9 +79,14 @@ func resourceCreate(ctx context.Context, data *schema.ResourceData, meta any) (i
 		)...,
 	)
 
+	data.SetId(resp.Id)
+
+	// Some fields are only set from calling a the read operation,
+	// so to keep the output consistent, this defers the remaining
+	// effort to read to get the data
 	return tfext.AppendDiagnostics(
 		issues,
-		tfext.AsErrorDiagnostics(encodeTerraform(resp, data))...,
+		resourceRead(ctx, data, meta)...,
 	)
 }
 
@@ -102,6 +107,12 @@ func resourceRead(ctx context.Context, data *schema.ResourceData, meta any) (iss
 		issues = tfext.AppendDiagnostics(issues, tfext.AsWarnDiagnostics(fmt.Errorf("detector is over mts limit"))...)
 	}
 
+	issues = tfext.AppendDiagnostics(issues,
+		tfext.AsErrorDiagnostics(
+			data.Set("url", pmeta.LoadApplicationURL(ctx, meta, AppPath, dt.Id, "edit")),
+		)...,
+	)
+
 	return tfext.AppendDiagnostics(
 		issues,
 		tfext.AsErrorDiagnostics(encodeTerraform(dt, data))...,
diff --git a/internal/definition/detector/resource_acceptance_test.go b/internal/definition/detector/resource_acceptance_test.go
index c0510c8b..4008f5cc 100644
--- a/internal/definition/detector/resource_acceptance_test.go
+++ b/internal/definition/detector/resource_acceptance_test.go
@@ -5,7 +5,6 @@ package detector_test
 
 import (
 	"testing"
-	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -29,7 +28,7 @@ func TestAcceptance(t *testing.T) {
 						resource.TestCheckResourceAttr("signalfx_detector.minimal", "name", "my minimal detector"),
 						resource.TestCheckResourceAttr("signalfx_detector.minimal", "program_text", "detect(when(const(1) > 1)).publish('HCF')\n"),
 					),
-					ExpectNonEmptyPlan: true,
+					ExpectNonEmptyPlan: false,
 				},
 			},
 		},
@@ -39,7 +38,6 @@ func TestAcceptance(t *testing.T) {
 				{
 					Config: tftest.LoadConfig("testdata/advanced_00.tf"),
 					Check: resource.ComposeAggregateTestCheckFunc(
-						tftest.DelayStateCheck(60*time.Second),
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "example detector"),
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "A detector made from terraform"),
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
@@ -50,10 +48,6 @@ func TestAcceptance(t *testing.T) {
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "30"),
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "15"),
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
-						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.%", "2"),
-						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 30m", "1000"),
-						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 5m", "1000"),
-
 						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "2"),
 
 						// Rule #1
@@ -86,77 +80,78 @@ func TestAcceptance(t *testing.T) {
 				{
 					Config: tftest.LoadConfig("testdata/advanced_01.tf"),
 					Check: resource.ComposeAggregateTestCheckFunc(
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "name", "max average delay UPDATED"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "description", "your application is slowER"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "timezone", "Europe/Paris"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.#", "3"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.0", "tag-1"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.1", "tag-2"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.2", "tag-3"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "teams.#", "0"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "max_delay", "60"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "min_delay", "15"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay",
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "max average delay UPDATED"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "your application is slowER"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.#", "3"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.0", "tag-1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.1", "tag-2"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.2", "tag-3"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "teams.#", "0"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "60"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "30"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector",
 							"time_range", "3600"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_data_markers", "true"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_event_lines", "true"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "disable_sampling", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_data_markers", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_event_lines", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "disable_sampling", "true"),
 
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "label_resolutions.%", "2"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "label_resolutions.Processing old messages 30m", "1000"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "label_resolutions.Processing old messages 5m", "1000"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.%", "2"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 30m", "1000"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 5m", "1000"),
 
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.#", "2"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "2"),
 
 						// Rule #1
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.description", "NEW maximum > 60 for 5m"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.notifications.0", "Email,foo-alerts@example.com"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.parameterized_body", ""),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.parameterized_subject", ""),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.severity", "Warning"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.runbook_url", "https://www.example.com"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.1.tip", "reboot it"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.description", "NEW maximum > 60 for 5m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_body", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_subject", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.severity", "Warning"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.runbook_url", "https://www.example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.tip", "reboot it"),
 
 						// Rule #2
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.description", "NEW maximum > 60 for 30m"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.detect_label", "Processing old messages 30m"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.disabled", "false"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.notifications.#", "1"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.parameterized_body", ""),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.parameterized_subject", ""),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.runbook_url", "https://www.example.com"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.severity", "Critical"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.tip", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.description", "NEW maximum > 60 for 30m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.detect_label", "Processing old messages 30m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.disabled", "false"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.#", "1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_body", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_subject", ""),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.runbook_url", "https://www.example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.severity", "Critical"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.tip", ""),
 					),
 				},
 				{
 					Config: tftest.LoadConfig("testdata/advanced_02.tf"),
 					Check: resource.ComposeAggregateTestCheckFunc(
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "name", "max average delay UPDATED"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "description", "your application is slowER"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "timezone", "Europe/Paris"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "tags.#", "0"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "teams.#", "0"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "max_delay", "60"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "min_delay", "30"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay",
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "max average delay UPDATED"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "your application is slowER"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.#", "0"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "teams.#", "0"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "60"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "30"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector",
 							"time_range", "3600"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\n"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_data_markers", "true"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "show_event_lines", "true"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "disable_sampling", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\n"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_data_markers", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_event_lines", "true"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "disable_sampling", "true"),
 
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.#", "1"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "1"),
 
 						// Rule #1
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.description", "NEW maximum > 60 for 5m"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.severity", "Warning"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.runbook_url", "https://www.example.com"),
-						resource.TestCheckResourceAttr("signalfx_detector.application_delay", "rule.0.tip", "reboot it"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.description", "NEW maximum > 60 for 5m"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.0", "Email,foo-alerts@example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.severity", "Warning"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.runbook_url", "https://www.example.com"),
+						resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.tip", "reboot it"),
 					),
+					ExpectNonEmptyPlan: true,
 				},
 			},
 		},
diff --git a/internal/definition/detector/resource_test.go b/internal/definition/detector/resource_test.go
index b307734d..780c9993 100644
--- a/internal/definition/detector/resource_test.go
+++ b/internal/definition/detector/resource_test.go
@@ -88,6 +88,34 @@ func TestResourceCreate(t *testing.T) {
 						DetectorOrigin:       req.DetectorOrigin,
 					})
 				},
+				"GET /v2/detector/id-01": func(w http.ResponseWriter, r *http.Request) {
+					_, _ = io.Copy(io.Discard, r.Body)
+					_ = r.Body.Close()
+
+					_ = json.NewEncoder(w).Encode(&detector.Detector{
+						Id:                "id-01",
+						Name:              "test detector",
+						Description:       "An example detector response",
+						AuthorizedWriters: &detector.AuthorizedWriters{},
+						TimeZone:          "Australia/Sydney",
+						MaxDelay:          common.AsPointer[int32](1000),
+						MinDelay:          common.AsPointer[int32](1000),
+						ProgramText:       `detect(when(data('*').count() < 1)).publish('no data')`,
+						OverMTSLimit:      false,
+						Rules: []*detector.Rule{
+							{
+								DetectLabel: "no data",
+								Notifications: []*notification.Notification{
+									{Type: "Team", Value: &notification.TeamNotification{Type: "Team", Team: "awesome-team"}},
+								},
+							},
+						},
+						Tags:                 []string{"tag-01"},
+						Teams:                []string{"team-01"},
+						DetectorOrigin:       "Standard",
+						VisualizationOptions: &detector.Visualization{},
+					})
+				},
 			}),
 			Encoder: encodeTerraform,
 			Decoder: decodeTerraform,
diff --git a/internal/definition/detector/schema.go b/internal/definition/detector/schema.go
index a5364a38..2de98118 100644
--- a/internal/definition/detector/schema.go
+++ b/internal/definition/detector/schema.go
@@ -310,7 +310,7 @@ func encodeTerraform(dt *detector.Detector, rd *schema.ResourceData) error {
 				errs = multierr.Append(errs, rd.Set("start_time", *t.Start))
 				errs = multierr.Append(errs, rd.Set("end_time", *t.End))
 			case t.Range != nil:
-				errs = multierr.Append(errs, rd.Set("time_range", *t.Range))
+				errs = multierr.Append(errs, rd.Set("time_range", *t.Range/1000))
 			}
 		}
 		labels := make([]map[string]any, 0, len(viz.PublishLabelOptions))
diff --git a/internal/definition/detector/testdata/advanced_02.tf b/internal/definition/detector/testdata/advanced_02.tf
index 6f1b78c2..a2048af7 100644
--- a/internal/definition/detector/testdata/advanced_02.tf
+++ b/internal/definition/detector/testdata/advanced_02.tf
@@ -1,4 +1,4 @@
-resource "signalfx_detector" "application_delay" {
+resource "signalfx_detector" "my_detector" {
   name        = "max average delay UPDATED"
   description = "your application is slowER"
   max_delay   = 60

From 3fedecf0fbfed16de13b58b84a7d023e41064847 Mon Sep 17 00:00:00 2001
From: Sean Marciniak <smarciniak@splunk.com>
Date: Thu, 24 Oct 2024 20:53:06 +1030
Subject: [PATCH 3/4] Improving overally quality

---
 internal/definition/detector/schema.go |  2 +-
 internal/definition/rule/encoding.go   |  3 +--
 internal/definition/team/schema.go     |  3 +--
 internal/tfextension/encoding.go       |  4 ++--
 internal/tfextension/values.go         | 23 -----------------------
 internal/tfextension/values_test.go    |  4 ----
 internal/tftest/state.go               | 18 ------------------
 internal/tftest/state_test.go          |  1 -
 8 files changed, 5 insertions(+), 53 deletions(-)
 delete mode 100644 internal/tfextension/values.go
 delete mode 100644 internal/tfextension/values_test.go
 delete mode 100644 internal/tftest/state.go
 delete mode 100644 internal/tftest/state_test.go

diff --git a/internal/definition/detector/schema.go b/internal/definition/detector/schema.go
index 2de98118..b037620e 100644
--- a/internal/definition/detector/schema.go
+++ b/internal/definition/detector/schema.go
@@ -194,7 +194,7 @@ func newSchema() map[string]*schema.Schema {
 	}
 }
 
-func decodeTerraform(rd tfext.Values) (*detector.Detector, error) {
+func decodeTerraform(rd *schema.ResourceData) (*detector.Detector, error) {
 	d := &detector.Detector{
 		Id:                rd.Id(),
 		Name:              rd.Get("name").(string),
diff --git a/internal/definition/rule/encoding.go b/internal/definition/rule/encoding.go
index 630fb1c2..a9e22d80 100644
--- a/internal/definition/rule/encoding.go
+++ b/internal/definition/rule/encoding.go
@@ -11,10 +11,9 @@ import (
 	"github.com/signalfx/signalfx-go/detector"
 
 	"github.com/splunk-terraform/terraform-provider-signalfx/internal/common"
-	tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension"
 )
 
-func DecodeTerraform(rd tfext.Values) ([]*detector.Rule, error) {
+func DecodeTerraform(rd *schema.ResourceData) ([]*detector.Rule, error) {
 	set, ok := rd.Get("rule").(*schema.Set)
 	if !ok {
 		return nil, errors.New("no field defined for rule")
diff --git a/internal/definition/team/schema.go b/internal/definition/team/schema.go
index 7efdd024..f0dd4c38 100644
--- a/internal/definition/team/schema.go
+++ b/internal/definition/team/schema.go
@@ -10,7 +10,6 @@ import (
 
 	"github.com/splunk-terraform/terraform-provider-signalfx/internal/check"
 	"github.com/splunk-terraform/terraform-provider-signalfx/internal/common"
-	tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension"
 )
 
 func newSchema() map[string]*schema.Schema {
@@ -93,7 +92,7 @@ func newSchema() map[string]*schema.Schema {
 	}
 }
 
-func decodeTerraform(rd tfext.Values) (*team.Team, error) {
+func decodeTerraform(rd *schema.ResourceData) (*team.Team, error) {
 	t := &team.Team{
 		Id:          rd.Id(),
 		Name:        rd.Get("name").(string),
diff --git a/internal/tfextension/encoding.go b/internal/tfextension/encoding.go
index a5c83855..b83666eb 100644
--- a/internal/tfextension/encoding.go
+++ b/internal/tfextension/encoding.go
@@ -12,7 +12,7 @@ type (
 	// the terraform state data into the expected API type to be used.
 	//
 	// This is to be used when interfacing with other packages.
-	DecodeTerraformFunc[T any] func(rd Values) (*T, error)
+	DecodeTerraformFunc[T any] func(rd *schema.ResourceData) (*T, error)
 
 	// EncodeTerraformFunc is used to write the API response data into
 	// the terraform state.
@@ -21,7 +21,7 @@ type (
 	EncodeTerraformFunc[T any] func(t *T, rd *schema.ResourceData) error
 )
 
-func NopDecodeTerraform[T any](Values) (*T, error) {
+func NopDecodeTerraform[T any](*schema.ResourceData) (*T, error) {
 	return nil, nil
 }
 
diff --git a/internal/tfextension/values.go b/internal/tfextension/values.go
deleted file mode 100644
index 026df022..00000000
--- a/internal/tfextension/values.go
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright Splunk, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
-package tfext
-
-import "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-
-// Values is a unified resource data type
-// to allow for high reusable methods in different scenarios.
-//
-// For method information, please refer to the defined types
-// below and check their API interface.
-type Values interface {
-	Id() string
-	Get(string) any
-	GetOk(string) (any, bool)
-	HasChanges(values ...string) bool
-}
-
-var (
-	_ Values = (*schema.ResourceData)(nil)
-	_ Values = (*schema.ResourceDiff)(nil)
-)
diff --git a/internal/tfextension/values_test.go b/internal/tfextension/values_test.go
deleted file mode 100644
index 4ba7f309..00000000
--- a/internal/tfextension/values_test.go
+++ /dev/null
@@ -1,4 +0,0 @@
-// Copyright Splunk, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
-package tfext
diff --git a/internal/tftest/state.go b/internal/tftest/state.go
deleted file mode 100644
index 167ce4ff..00000000
--- a/internal/tftest/state.go
+++ /dev/null
@@ -1,18 +0,0 @@
-package tftest
-
-import (
-	"time"
-
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
-)
-
-// DelayStateCheck should be used in times when the services
-// need some additional processing time to ensure that all the expected
-// values are populated.
-func DelayStateCheck(delay time.Duration) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		<-time.After(delay)
-		return nil
-	}
-}
diff --git a/internal/tftest/state_test.go b/internal/tftest/state_test.go
deleted file mode 100644
index 639d1257..00000000
--- a/internal/tftest/state_test.go
+++ /dev/null
@@ -1 +0,0 @@
-package tftest

From 89362b2c56b08d8adfa0cf44a96e62d4275b3090 Mon Sep 17 00:00:00 2001
From: Sean Marciniak <30928402+MovieStoreGuy@users.noreply.github.com>
Date: Fri, 25 Oct 2024 09:46:13 +1030
Subject: [PATCH 4/4] Update internal/common/notification_list.go

Co-authored-by: Antoine Toulme <antoine@toulme.name>
---
 internal/common/notification_list.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/internal/common/notification_list.go b/internal/common/notification_list.go
index 48718d3b..42b89a88 100644
--- a/internal/common/notification_list.go
+++ b/internal/common/notification_list.go
@@ -11,7 +11,6 @@ func NewNotificationList(items []any) ([]*notification.Notification, error) {
 	if len(items) == 0 {
 		return nil, nil
 	}
-
 	values := make([]*notification.Notification, len(items))
 	for i, v := range items {
 		n, err := NewNotificationFromString(v.(string))