Commit d5be0b50 authored by syasonik's avatar syasonik

Make dashboard embedded check more explicit

When a metrics dashboard is requested, the key :embedded
in the query params is used to determine whether a request
is for an embed or a full dashboard. Instead of relying
on just the presence of the param, this swaps the check
over to whether the param's value represents true/false.
parent 202f290b
......@@ -5,6 +5,10 @@
module Metrics
module Dashboard
class BaseEmbedService < ::Metrics::Dashboard::BaseService
def self.embedded?(embed_param)
ActiveModel::Type::Boolean.new.cast(embed_param)
end
def cache_key
"dynamic_metrics_dashboard_#{identifiers}"
end
......
......@@ -18,7 +18,7 @@ module Metrics
# custom metrics from the DB.
def valid_params?(params)
[
params[:embedded],
embedded?(params[:embedded]),
valid_dashboard?(params[:dashboard_path]),
valid_group_title?(params[:group]),
params[:title].present?,
......
......@@ -22,7 +22,7 @@ module Metrics
class << self
def valid_params?(params)
params[:embedded].present?
embedded?(params[:embedded])
end
end
......
......@@ -22,7 +22,7 @@ module Metrics
# for additional info on defining custom dashboards.
def valid_params?(params)
[
params[:embedded],
embedded?(params[:embedded]),
params[:group].present?,
params[:title].present?,
params[:y_label]
......
......@@ -6,7 +6,7 @@
# Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards.
module Metrics
module Dashboard
class GrafanaMetricEmbedService < ::Metrics::Dashboard::BaseService
class GrafanaMetricEmbedService < ::Metrics::Dashboard::BaseEmbedService
include ReactiveCaching
SEQUENCE = [
......@@ -24,7 +24,7 @@ module Metrics
# to uniquely identify a grafana dashboard.
def valid_params?(params)
[
params[:embedded],
embedded?(params[:embedded]),
params[:grafana_url]
].all?
end
......
......@@ -18,7 +18,7 @@ module Metrics
# custom metrics from the DB.
def valid_params?(params)
[
params[:embedded],
embedded?(params[:embedded]),
params[:prometheus_alert_id].is_a?(Integer)
].all?
end
......
......@@ -29,12 +29,18 @@ describe Metrics::Dashboard::GitlabAlertEmbedService do
it { is_expected.to be_truthy }
context 'not embedded' do
context 'missing embedded' do
let(:params) { valid_params.except(:embedded) }
it { is_expected.to be_falsey }
end
context 'not embedded' do
let(:params) { valid_params.merge(embedded: 'false') }
it { is_expected.to be_falsey }
end
context 'missing alert id' do
let(:params) { valid_params.except(:prometheus_alert_id) }
......
......@@ -35,12 +35,18 @@ describe Metrics::Dashboard::CustomMetricEmbedService do
it { is_expected.to be_truthy }
context 'not embedded' do
context 'missing embedded' do
let(:params) { valid_params.except(:embedded) }
it { is_expected.to be_falsey }
end
context 'not embedded' do
let(:params) { valid_params.merge(embedded: 'false') }
it { is_expected.to be_falsey }
end
context 'non-system dashboard' do
let(:dashboard_path) { '.gitlab/dashboards/test.yml' }
......
......@@ -27,7 +27,7 @@ describe Metrics::Dashboard::DefaultEmbedService, :use_clean_rails_memory_store_
end
context 'not embedded' do
let(:params) { { embedded: false } }
let(:params) { { embedded: 'false' } }
it { is_expected.to be_falsey }
end
......
......@@ -35,12 +35,18 @@ describe Metrics::Dashboard::DynamicEmbedService, :use_clean_rails_memory_store_
it { is_expected.to be_truthy }
context 'not embedded' do
context 'missing embedded' do
let(:params) { valid_params.except(:embedded) }
it { is_expected.to be_falsey }
end
context 'not embedded' do
let(:params) { valid_params.merge(embedded: 'false') }
it { is_expected.to be_falsey }
end
context 'undefined dashboard' do
let(:params) { valid_params.except(:dashboard_path) }
......
......@@ -28,12 +28,18 @@ describe Metrics::Dashboard::GrafanaMetricEmbedService do
it { is_expected.to be_truthy }
context 'not embedded' do
context 'missing embedded' do
let(:params) { valid_params.except(:embedded) }
it { is_expected.to be_falsey }
end
context 'not embedded' do
let(:params) { valid_params.merge(embedded: 'false') }
it { is_expected.to be_falsey }
end
context 'undefined grafana_url' do
let(:params) { valid_params.except(:grafana_url) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment