From a0312f6af2e6ad7b8c8c319100aa090adcb6638f Mon Sep 17 00:00:00 2001 From: Puskar Basu Date: Thu, 28 Nov 2024 19:33:03 +0530 Subject: [PATCH 1/5] Add primary_key property to DashboardTableColumn resource to specify primary keys in a table --- internal/resources/dashboard_table_column.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/resources/dashboard_table_column.go b/internal/resources/dashboard_table_column.go index 96021853..564e888a 100644 --- a/internal/resources/dashboard_table_column.go +++ b/internal/resources/dashboard_table_column.go @@ -3,10 +3,11 @@ package resources import "github.com/turbot/pipe-fittings/utils" type DashboardTableColumn struct { - Name string `hcl:"name,label" json:"name" snapshot:"name"` - Display *string `cty:"display" hcl:"display" json:"display,omitempty" snapshot:"display"` - Wrap *string `cty:"wrap" hcl:"wrap" json:"wrap,omitempty" snapshot:"wrap"` - HREF *string `cty:"href" hcl:"href" json:"href,omitempty" snapshot:"href"` + Name string `hcl:"name,label" json:"name" snapshot:"name"` + Display *string `cty:"display" hcl:"display" json:"display,omitempty" snapshot:"display"` + Wrap *string `cty:"wrap" hcl:"wrap" json:"wrap,omitempty" snapshot:"wrap"` + HREF *string `cty:"href" hcl:"href" json:"href,omitempty" snapshot:"href"` + PrimaryKey *bool `cty:"primary_key" hcl:"primary_key" json:"primary_key,omitempty" snapshot:"primary_key"` } func (c DashboardTableColumn) Equals(other *DashboardTableColumn) bool { @@ -17,5 +18,6 @@ func (c DashboardTableColumn) Equals(other *DashboardTableColumn) bool { return utils.SafeStringsEqual(c.Name, other.Name) && utils.SafeStringsEqual(c.Display, other.Display) && utils.SafeStringsEqual(c.Wrap, other.Wrap) && - utils.SafeStringsEqual(c.HREF, other.HREF) + utils.SafeStringsEqual(c.HREF, other.HREF) && + utils.SafeBoolEqual(c.PrimaryKey, other.PrimaryKey) } From fd1506d21a596acb75e997f09cf24b5dcfa2330a Mon Sep 17 00:00:00 2001 From: Puskar Basu Date: Thu, 28 Nov 2024 19:34:14 +0530 Subject: [PATCH 2/5] add test/example dashboard and snapshot --- .../mods/dashboard_table/dashboard.pp | 36 ++++++ .../test_data/mods/dashboard_table/mod.pp | 4 + ...rd.testing_card_blocks.20241128T192603.pps | 108 ++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 tests/acceptance/test_data/mods/dashboard_table/dashboard.pp create mode 100644 tests/acceptance/test_data/mods/dashboard_table/mod.pp create mode 100644 tests/acceptance/test_data/snapshots/dashboard_table.dashboard.testing_card_blocks.20241128T192603.pps diff --git a/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp b/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp new file mode 100644 index 00000000..5d7de0d5 --- /dev/null +++ b/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp @@ -0,0 +1,36 @@ +dashboard "testing_card_blocks" { + title = "Testing card blocks" + + container { + table { + title = "Employee table" + width = 4 + + sql = <<-EOQ + SELECT + 011 AS id, + 'dwight' AS name, + 'manager' AS role + UNION ALL + SELECT + 012 AS id, + 'jim' AS name, + 'developer' AS role + UNION ALL + SELECT + 013 AS id, + 'pam' AS name, + 'designer' AS role; + EOQ + + column "id" { + display = "all" + primary_key = true + } + + column "name" { + display = "all" + } + } + } +} \ No newline at end of file diff --git a/tests/acceptance/test_data/mods/dashboard_table/mod.pp b/tests/acceptance/test_data/mods/dashboard_table/mod.pp new file mode 100644 index 00000000..62ef3873 --- /dev/null +++ b/tests/acceptance/test_data/mods/dashboard_table/mod.pp @@ -0,0 +1,4 @@ +mod "dashboard_table"{ + title = "Dashboard using table blocks" + description = "Dashboard for testing table blocks" +} \ No newline at end of file diff --git a/tests/acceptance/test_data/snapshots/dashboard_table.dashboard.testing_card_blocks.20241128T192603.pps b/tests/acceptance/test_data/snapshots/dashboard_table.dashboard.testing_card_blocks.20241128T192603.pps new file mode 100644 index 00000000..6e6fe3f2 --- /dev/null +++ b/tests/acceptance/test_data/snapshots/dashboard_table.dashboard.testing_card_blocks.20241128T192603.pps @@ -0,0 +1,108 @@ +{ + "schema_version": "20240607", + "inputs": {}, + "variables": {}, + "start_time": "2024-11-28T19:25:55.998731+05:30", + "end_time": "2024-11-28T19:25:56.032481+05:30", + "layout": { + "name": "dashboard_table.dashboard.testing_card_blocks", + "children": [ + { + "name": "dashboard_table.container.dashboard_testing_card_blocks_anonymous_container_0", + "children": [ + { + "name": "dashboard_table.table.container_dashboard_testing_card_blocks_anonymous_container_0_anonymous_table_0", + "panel_type": "table" + } + ], + "panel_type": "container" + } + ], + "panel_type": "dashboard" + }, + "panels": { + "dashboard_table.container.dashboard_testing_card_blocks_anonymous_container_0": { + "dashboard": "dashboard_table.dashboard.testing_card_blocks", + "name": "dashboard_table.container.dashboard_testing_card_blocks_anonymous_container_0", + "panel_type": "container", + "status": "complete" + }, + "dashboard_table.dashboard.testing_card_blocks": { + "dashboard": "dashboard_table.dashboard.testing_card_blocks", + "name": "dashboard_table.dashboard.testing_card_blocks", + "panel_type": "dashboard", + "status": "complete", + "title": "Testing card blocks" + }, + "dashboard_table.table.container_dashboard_testing_card_blocks_anonymous_container_0_anonymous_table_0": { + "dashboard": "dashboard_table.dashboard.testing_card_blocks", + "name": "dashboard_table.table.container_dashboard_testing_card_blocks_anonymous_container_0_anonymous_table_0", + "panel_type": "table", + "status": "complete", + "title": "Employee table", + "width": 4, + "data": { + "columns": [ + { + "name": "id", + "data_type": "INT4" + }, + { + "name": "name", + "data_type": "TEXT" + }, + { + "name": "role", + "data_type": "TEXT" + } + ], + "rows": [ + { + "id": 11, + "name": "dwight", + "role": "manager" + }, + { + "id": 12, + "name": "jim", + "role": "developer" + }, + { + "id": 13, + "name": "pam", + "role": "designer" + } + ] + }, + "properties": { + "columns": { + "id": { + "name": "id", + "display": "all", + "primary_key": true + }, + "name": { + "name": "name", + "display": "all" + } + }, + "name": "container_dashboard_testing_card_blocks_anonymous_container_0_anonymous_table_0" + } + } + }, + "metadata": { + "view": { + "group_by": [ + { + "type": "benchmark" + }, + { + "type": "control" + }, + { + "type": "result" + } + ] + } + } +} \ No newline at end of file From 37d08281905df0ac800d6cd0049ab6a4c68f1e5d Mon Sep 17 00:00:00 2001 From: Puskar Basu Date: Fri, 29 Nov 2024 19:02:16 +0530 Subject: [PATCH 3/5] update enums and add validation --- internal/resources/dashboard_table.go | 11 ++++ internal/resources/dashboard_table_column.go | 51 ++++++++++++++++--- .../mods/dashboard_table/dashboard.pp | 6 +-- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/internal/resources/dashboard_table.go b/internal/resources/dashboard_table.go index 25fe5907..f79d27e0 100644 --- a/internal/resources/dashboard_table.go +++ b/internal/resources/dashboard_table.go @@ -77,11 +77,22 @@ func (t *DashboardTable) Equals(other *DashboardTable) bool { // OnDecoded implements HclResource func (t *DashboardTable) OnDecoded(block *hcl.Block, resourceMapProvider modconfig.ModResourcesProvider) hcl.Diagnostics { t.SetBaseProperties() + var diags hcl.Diagnostics // populate columns map if len(t.ColumnList) > 0 { t.Columns = make(map[string]*DashboardTableColumn, len(t.ColumnList)) for _, c := range t.ColumnList { t.Columns[c.Name] = c + // validate column properties + if err := c.Validate(); err != nil { + // append the validation error to diagnostics + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Detail: err.Error(), + Subject: block.DefRange.Ptr(), + }) + return diags + } } } return t.QueryProviderImpl.OnDecoded(block, resourceMapProvider) diff --git a/internal/resources/dashboard_table_column.go b/internal/resources/dashboard_table_column.go index 564e888a..ea4e7415 100644 --- a/internal/resources/dashboard_table_column.go +++ b/internal/resources/dashboard_table_column.go @@ -1,13 +1,50 @@ package resources -import "github.com/turbot/pipe-fittings/utils" +import ( + "github.com/turbot/pipe-fittings/sperr" + "github.com/turbot/pipe-fittings/utils" +) + +type DiffMode string + +const ( + DiffModeInclude DiffMode = "include" + DiffModeExclude DiffMode = "exclude" + DiffModeKey DiffMode = "key" +) type DashboardTableColumn struct { - Name string `hcl:"name,label" json:"name" snapshot:"name"` - Display *string `cty:"display" hcl:"display" json:"display,omitempty" snapshot:"display"` - Wrap *string `cty:"wrap" hcl:"wrap" json:"wrap,omitempty" snapshot:"wrap"` - HREF *string `cty:"href" hcl:"href" json:"href,omitempty" snapshot:"href"` - PrimaryKey *bool `cty:"primary_key" hcl:"primary_key" json:"primary_key,omitempty" snapshot:"primary_key"` + Name string `hcl:"name,label" json:"name" snapshot:"name"` + Display *string `cty:"display" hcl:"display" json:"display,omitempty" snapshot:"display"` + Wrap *string `cty:"wrap" hcl:"wrap" json:"wrap,omitempty" snapshot:"wrap"` + HREF *string `cty:"href" hcl:"href" json:"href,omitempty" snapshot:"href"` + DiffMode *DiffMode `cty:"diff_mode" hcl:"diff_mode,optional" json:"diff_mode,omitempty" snapshot:"diff_mode"` +} + +// Validate checks the validity of the column's properties and sets default values. +func (c *DashboardTableColumn) Validate() error { + // validate Display + if c.Display != nil && *c.Display != "all" && *c.Display != "none" { + return sperr.New(`invalid value for display: %s (allowed values: 'all', 'none')`, *c.Display) + } + + // validate Wrap + if c.Wrap != nil && *c.Wrap != "all" && *c.Wrap != "none" { + return sperr.New(`invalid value for wrap: %s (allowed values: 'all', 'none')`, *c.Wrap) + } + + // Set default DiffMode if not set + if c.DiffMode == nil { + defaultDiffMode := DiffModeInclude + c.DiffMode = &defaultDiffMode + } + + // validate DiffMode + if c.DiffMode != nil && *c.DiffMode != DiffModeInclude && *c.DiffMode != DiffModeExclude && *c.DiffMode != DiffModeKey { + return sperr.New(`invalid value for diff_mode: %s (allowed values: 'include', 'exclude', 'key')`, *c.DiffMode) + } + + return nil } func (c DashboardTableColumn) Equals(other *DashboardTableColumn) bool { @@ -19,5 +56,5 @@ func (c DashboardTableColumn) Equals(other *DashboardTableColumn) bool { utils.SafeStringsEqual(c.Display, other.Display) && utils.SafeStringsEqual(c.Wrap, other.Wrap) && utils.SafeStringsEqual(c.HREF, other.HREF) && - utils.SafeBoolEqual(c.PrimaryKey, other.PrimaryKey) + utils.SafeStringsEqual(c.DiffMode, other.DiffMode) } diff --git a/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp b/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp index 5d7de0d5..eac4e216 100644 --- a/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp +++ b/tests/acceptance/test_data/mods/dashboard_table/dashboard.pp @@ -1,5 +1,5 @@ -dashboard "testing_card_blocks" { - title = "Testing card blocks" +dashboard "testing_tables" { + title = "Testing table blocks" container { table { @@ -25,7 +25,7 @@ column "id" { display = "all" - primary_key = true + diff_mode = "key" } column "name" { From fcbde93157dd979286cc280c22f0cd6ebdf68f2e Mon Sep 17 00:00:00 2001 From: Puskar Basu Date: Fri, 29 Nov 2024 19:39:13 +0530 Subject: [PATCH 4/5] use map[string]struct to compare display, wrap values --- internal/resources/dashboard_table.go | 6 +++- internal/resources/dashboard_table_column.go | 31 +++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/internal/resources/dashboard_table.go b/internal/resources/dashboard_table.go index f79d27e0..f3304c49 100644 --- a/internal/resources/dashboard_table.go +++ b/internal/resources/dashboard_table.go @@ -1,10 +1,13 @@ package resources import ( + "fmt" + "github.com/hashicorp/hcl/v2" typehelpers "github.com/turbot/go-kit/types" "github.com/turbot/pipe-fittings/constants" "github.com/turbot/pipe-fittings/cty_helpers" + "github.com/turbot/pipe-fittings/hclhelpers" "github.com/turbot/pipe-fittings/modconfig" "github.com/turbot/pipe-fittings/printers" "github.com/turbot/pipe-fittings/schema" @@ -89,7 +92,8 @@ func (t *DashboardTable) OnDecoded(block *hcl.Block, resourceMapProvider modconf diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Detail: err.Error(), - Subject: block.DefRange.Ptr(), + Summary: fmt.Sprintf("column '%s' has invalid value", c.Name), + Subject: hclhelpers.BlockRangePointer(block), }) return diags } diff --git a/internal/resources/dashboard_table_column.go b/internal/resources/dashboard_table_column.go index ea4e7415..7c99e021 100644 --- a/internal/resources/dashboard_table_column.go +++ b/internal/resources/dashboard_table_column.go @@ -13,6 +13,19 @@ const ( DiffModeKey DiffMode = "key" ) +// all and none are valid values for display and wrap +var validDisplayAndWrap = map[string]struct{}{ + "all": {}, + "none": {}, +} + +// include, exclude and key are valid values for diff_mode +var validDiffMode = map[string]struct{}{ + string(DiffModeInclude): {}, + string(DiffModeExclude): {}, + string(DiffModeKey): {}, +} + type DashboardTableColumn struct { Name string `hcl:"name,label" json:"name" snapshot:"name"` Display *string `cty:"display" hcl:"display" json:"display,omitempty" snapshot:"display"` @@ -23,14 +36,18 @@ type DashboardTableColumn struct { // Validate checks the validity of the column's properties and sets default values. func (c *DashboardTableColumn) Validate() error { - // validate Display - if c.Display != nil && *c.Display != "all" && *c.Display != "none" { - return sperr.New(`invalid value for display: %s (allowed values: 'all', 'none')`, *c.Display) + // validate display + if c.Display != nil { + if _, ok := validDisplayAndWrap[*c.Display]; !ok { + return sperr.New(`invalid value for display: %s (allowed values: 'all', 'none')`, *c.Display) + } } - // validate Wrap - if c.Wrap != nil && *c.Wrap != "all" && *c.Wrap != "none" { - return sperr.New(`invalid value for wrap: %s (allowed values: 'all', 'none')`, *c.Wrap) + // validate wrap + if c.Wrap != nil { + if _, ok := validDisplayAndWrap[*c.Wrap]; !ok { + return sperr.New(`invalid value for wrap: %s (allowed values: 'all', 'none')`, *c.Wrap) + } } // Set default DiffMode if not set @@ -40,7 +57,7 @@ func (c *DashboardTableColumn) Validate() error { } // validate DiffMode - if c.DiffMode != nil && *c.DiffMode != DiffModeInclude && *c.DiffMode != DiffModeExclude && *c.DiffMode != DiffModeKey { + if _, ok := validDiffMode[string(*c.DiffMode)]; !ok { return sperr.New(`invalid value for diff_mode: %s (allowed values: 'include', 'exclude', 'key')`, *c.DiffMode) } From cb1685cd846d0e15d466b27682d05a6e16d28fe5 Mon Sep 17 00:00:00 2001 From: Puskar Basu Date: Fri, 29 Nov 2024 20:28:49 +0530 Subject: [PATCH 5/5] update expected snapshot --- .../snapshots/expected_sps_testing_dashboard_inputs.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/acceptance/test_data/snapshots/expected_sps_testing_dashboard_inputs.json b/tests/acceptance/test_data/snapshots/expected_sps_testing_dashboard_inputs.json index 7e837151..980d321b 100644 --- a/tests/acceptance/test_data/snapshots/expected_sps_testing_dashboard_inputs.json +++ b/tests/acceptance/test_data/snapshots/expected_sps_testing_dashboard_inputs.json @@ -67,6 +67,7 @@ "properties": { "columns": { "Alternative Names": { + "diff_mode": "include", "name": "Alternative Names", "wrap": "all" }