Merge branch 'sy-embedded-refactor' into 'master'

Make metrics dashboard endpoint embedded check more explicit

See merge request gitlab-org/gitlab!26329

app/services/metrics/dashboard/base_embed_service.rb

@@ -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

app/services/metrics/dashboard/custom_metric_embed_service.rb

@@ -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?,

app/services/metrics/dashboard/default_embed_service.rb

@@ -22,7 +22,7 @@ module Metrics
 
       class << self
         def valid_params?(params)
-          params[:embedded].present?
+          embedded?(params[:embedded])
         end
       end
 

app/services/metrics/dashboard/dynamic_embed_service.rb

@@ -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]

app/services/metrics/dashboard/grafana_metric_embed_service.rb

@@ -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

ee/app/services/metrics/dashboard/gitlab_alert_embed_service.rb

@@ -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

ee/spec/services/metrics/dashboard/gitlab_alert_embed_service_spec.rb

@@ -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) }
 

spec/services/metrics/dashboard/custom_metric_embed_service_spec.rb

@@ -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' }
 

spec/services/metrics/dashboard/default_embed_service_spec.rb

@@ -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

spec/services/metrics/dashboard/dynamic_embed_service_spec.rb

@@ -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) }
 

spec/services/metrics/dashboard/grafana_metric_embed_service_spec.rb

@@ -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) }