Skip to content

Commit

Permalink
fix symbol validation not working in dashboards
Browse files Browse the repository at this point in the history
  • Loading branch information
grosser committed Nov 2, 2023
1 parent c1112d6 commit 29d2039
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 19 deletions.
2 changes: 1 addition & 1 deletion lib/kennel/models/record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def validate_json(data)
bad = Kennel::Utils.all_keys(data).grep_v(Symbol).sort.uniq
return if bad.empty?
invalid!(
:hash_keys_must_be_symbols,
OptionalValidations::UNIGNORABLE,
"Only use Symbols as hash keys to avoid permanent diffs when updating.\n" \
"Change these keys to be symbols (usually 'foo' => 1 --> 'foo': 1)\n" \
"#{bad.map(&:inspect).join("\n")}"
Expand Down
2 changes: 1 addition & 1 deletion lib/kennel/optional_validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def initialize(...)
end

def invalid!(tag, message)
validation_errors << ValidationMessage.new(tag || OptionalValidations::UNIGNORABLE, message)
validation_errors << ValidationMessage.new(tag, message)
end

def self.valid?(parts)
Expand Down
2 changes: 2 additions & 0 deletions lib/kennel/template_variables.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def render_template_variables
# check for queries that do not use the variables and would be misleading
# TODO: do the same check for apm_query and their group_by
def validate_json(data)
super

variables = (data[:template_variables] || []).map { |v| "$#{v.fetch(:name)}" }
return if variables.empty?

Expand Down
1 change: 1 addition & 0 deletions test/kennel/models/dashboard_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
describe Kennel::Models::Dashboard do
define_test_classes

# TODO: move into define_test_classes
class TestDashboard < Kennel::Models::Dashboard
end

Expand Down
35 changes: 18 additions & 17 deletions test/kennel/template_variables_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,66 +29,67 @@ def var(value)
end

describe "#validate_json" do
let(:errors) { [] }

let(:item) do
item = Object.new
item.extend Kennel::TemplateVariables
item.stubs(:invalid!).with { |_tag, err| errors << err }
item
end
let(:errors) { item.validation_errors }
let(:error_tags) { errors.map(&:tag) }
let(:item) { Kennel::Models::Dashboard.new(TestProject.new) }

def validate(variable_names, widgets)
data = {
tags: [],
template_variables: variable_names.map { |v| { name: v } },
widgets: widgets
}

item.send(:validate_json, data)
end

it "is valid when empty" do
validate [], []
errors.must_be_empty
error_tags.must_equal []
end

it "is valid when vars are empty" do
validate [], [{ definition: { requests: [{ q: "x" }] } }]
errors.must_be_empty
error_tags.must_equal []
end

it "is valid when vars are used" do
validate ["a"], [{ definition: { requests: [{ q: "$a" }] } }]
errors.must_be_empty
error_tags.must_equal []
end

it "is invalid when vars are not used" do
validate ["a"], [{ definition: { requests: [{ q: "$b" }] } }]
errors.length.must_equal 1
error_tags.must_equal [:queries_must_use_template_variables]
end

it "is invalid when some vars are not used" do
validate ["a", "b"], [{ definition: { requests: [{ q: "$b" }] } }]
errors.length.must_equal 1
error_tags.must_equal [:queries_must_use_template_variables]
end

it "is valid when all vars are used" do
validate ["a", "b"], [{ definition: { requests: [{ q: "$a,$b" }] } }]
error_tags.must_equal []
end

it "is invalid when nested vars are not used" do
validate ["a"], [{ definition: { widgets: [{ definition: { requests: [{ q: "$b" }] } }] } }]
errors.length.must_equal 1
error_tags.must_equal [:queries_must_use_template_variables]
end

it "works with hostmap widgets" do
validate ["a"], [{ definition: { requests: { fill: { q: "x" } } } }]
errors.length.must_equal 1
error_tags.must_equal [:queries_must_use_template_variables]
end

it "works with new api format" do
validate ["a"], [{ definition: { requests: [{ queries: [{ query: "x" }] }] } }]
errors.length.must_equal 1
error_tags.must_equal [:queries_must_use_template_variables]
end

it "still calls existing validations" do
validate [], [{ "definition" => { requests: [{ q: "x" }] } }]
error_tags.must_equal [:unignorable]
end
end
end

0 comments on commit 29d2039

Please sign in to comment.