Skip to content

Conversation

@tobio
Copy link
Member

@tobio tobio commented Feb 3, 2026

Related to #650

@tobio tobio requested a review from dimuon February 3, 2026 01:53
@tobio tobio self-assigned this Feb 3, 2026
Copilot AI review requested due to automatic review settings February 3, 2026 01:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for tag cloud panels in Kibana dashboards, allowing users to visualize word frequency data. The implementation follows the same pattern as existing XY chart panels.

Changes:

  • Adds a new tagcloud_config panel configuration type alongside existing markdown_config and xy_chart_config
  • Implements schema definitions, model converters, and API mappers for tagcloud panels
  • Includes comprehensive test coverage with basic and filtered tagcloud scenarios

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/kibana/dashboard/schema.go Adds tagcloud schema definitions with JSON field defaults, updates panel schema to include tagcloud_config with mutual exclusivity validators
internal/kibana/dashboard/models_tagcloud_panel.go Implements tagcloud panel converter with fromAPI/toAPI transformations
internal/kibana/dashboard/models_tagcloud_panel_test.go Adds unit tests for tagcloud config conversions and JSON field validation
internal/kibana/dashboard/models_panels.go Registers tagcloud converter in the panel converters list
internal/kibana/dashboard/models_filter_simple.go Extracts shared filter simple model to separate file for reuse
internal/kibana/dashboard/models_filter_simple_test.go Moves filter simple tests to new file
internal/kibana/dashboard/models_search_filter.go Extracts shared search filter model to separate file for reuse
internal/kibana/dashboard/models_search_filter_test.go Moves search filter tests to new file
internal/kibana/dashboard/models_xy_chart_panel.go Removes filter models now extracted to separate files
internal/kibana/dashboard/models_xy_chart_panel_test.go Removes filter model tests now in separate files
internal/kibana/dashboard/acc_test.go Adds acceptance tests for basic and filtered tagcloud panels
internal/kibana/dashboard/testdata/.../basic/main.tf Provides basic tagcloud panel test configuration
internal/kibana/dashboard/testdata/.../with_filters/main.tf Provides filtered tagcloud panel test configuration

},
},
"metric": schema.StringAttribute{
MarkdownDescription: "Metric configuration as JSON. Can be a field metric operation (count, unique count, min/max/avg/median/std dev, sum, last value, percentile, percentile ranks), a pipeline operation (differences, moving average, cumulative sum, counter rate), or a formula operation.",
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The markdown description for the metric attribute lists operation types inconsistently. It should use consistent separators (either commas or slashes throughout, not both) and maintain parallel structure. Consider: "Can be a field metric operation (count, unique count, min, max, avg, median, std dev, sum, last value, percentile, percentile ranks), a pipeline operation (differences, moving average, cumulative sum, counter rate), or a formula operation."

Suggested change
MarkdownDescription: "Metric configuration as JSON. Can be a field metric operation (count, unique count, min/max/avg/median/std dev, sum, last value, percentile, percentile ranks), a pipeline operation (differences, moving average, cumulative sum, counter rate), or a formula operation.",
MarkdownDescription: "Metric configuration as JSON. Can be a field metric operation (count, unique count, min, max, avg, median, std dev, sum, last value, percentile, percentile ranks), a pipeline operation (differences, moving average, cumulative sum, counter rate), or a formula operation.",

Copilot uses AI. Check for mistakes.
return diags
}

// Create the nested Config1 structure
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment refers to "Config1" but this is an implementation detail of the generated API types. Consider making this comment more descriptive about what this structure represents, e.g., "Create the nested config structure for the tagcloud panel attributes".

Suggested change
// Create the nested Config1 structure
// Create the nested dashboard panel config structure for the tagcloud attributes

Copilot uses AI. Check for mistakes.
MarkdownDescription: "Configuration for a tagcloud chart panel. Mutually exclusive with `markdown_config`, `xy_chart_config`, and `config_json`. Tag clouds visualize word frequency.",
Optional: true,
Attributes: getTagcloudSchema(),
Validators: []validator.Object{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to require at least one of markdown_config, xy_chart_config, tagcloud_config or config_json?

Is this TF config is valid?

panels = [{
  type = "lens"
  grid = { x = 0, y = 0, w = 24, h = 15 }
  # No markdown_config, xy_chart_config, tagcloud_config, or config_json
}]

func newTagcloudPanelConfigConverter() tagcloudPanelConfigConverter {
return tagcloudPanelConfigConverter{
lensPanelConfigConverter: lensPanelConfigConverter{
visualizationType: "tagcloud",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use enum here as we do it for XY already?

Suggested change
visualizationType: "tagcloud",
visualizationType: string(kbapi.TagcloudNoESQLTypeTagcloud),

var api kbapi.TagcloudNoESQL

// Set type to "tagcloud"
api.Type = "tagcloud"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
api.Type = "tagcloud"
api.Type = kbapi.TagcloudNoESQLTypeTagcloud

assert.Equal(t, "tagcloud", converter.visualizationType)
}

func Test_tagcloudConfigModel_fromAPI_toAPI(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't cover filters array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants