Merge branch...

Merge branch '39505-extend-the-ability-to-clone-a-dashboard-so-it-would-copy-the-custom-metrics' into 'master'

Extend the ability to clone a dashboard so it would copy the custom metrics

See merge request gitlab-org/gitlab!21923

app/services/metrics/dashboard/clone_dashboard_service.rb

@@ -8,8 +8,18 @@ module Metrics
       ALLOWED_FILE_TYPE = '.yml'
       USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT
 
-      def self.allowed_dashboard_templates
-        @allowed_dashboard_templates ||= Set[::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH].freeze
+      class << self
+        def allowed_dashboard_templates
+          @allowed_dashboard_templates ||= Set[::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH].freeze
+        end
+
+        def sequences
+          @sequences ||= {
+            ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
+                                                                             ::Gitlab::Metrics::Dashboard::Stages::ProjectMetricsInserter,
+                                                                             ::Gitlab::Metrics::Dashboard::Stages::Sorter].freeze
+          }.freeze
+        end
       end
 
       def execute
@@ -92,7 +102,9 @@ module Metrics
       end
 
       def new_dashboard_content
-        File.read(Rails.root.join(dashboard_template))
+        ::Gitlab::Metrics::Dashboard::Processor
+          .new(project, raw_dashboard, sequence, {})
+          .process.deep_stringify_keys.to_yaml
       end
 
       def repository
@@ -106,6 +118,14 @@ module Metrics
           result
         end
       end
+
+      def raw_dashboard
+        YAML.safe_load(File.read(Rails.root.join(dashboard_template)))
+      end
+
+      def sequence
+        self.class.sequences[dashboard_template]
+      end
     end
   end
 end

changelogs/unreleased/39505-extend-the-ability-to-clone-a-dashboard-so-it-would-copy-the-custom.yml

+---
+title: Extends 'Duplicate dashboard' feature, by including custom metrics added to
+  GitLab-defined dashboards.
+merge_request: 21923
+author:
+type: added

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

@@ -15,6 +15,12 @@ module EE
           def allowed_dashboard_templates
             @allowed_dashboard_templates ||= (Set[::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH] + super).freeze
           end
+
+          override :sequences
+          def sequences
+            super.merge(::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH => [::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
+                                                                                          ::Gitlab::Metrics::Dashboard::Stages::Sorter].freeze)
+          end
         end
       end
     end

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

@@ -3,11 +3,15 @@
 require 'spec_helper'
 
 describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_store_caching do
+  STAGES = ::Gitlab::Metrics::Dashboard::Stages
+
   set(:user) { create(:user) }
   set(:project) { create(:project, :repository) }
   set(:environment) { create(:environment, project: project) }
 
   describe '#execute' do
+    subject(:service_call) { described_class.new(project, user, params).execute }
+
     context 'with rights to push to the repository' do
       before do
         project.add_maintainer(user)
@@ -17,37 +21,18 @@ describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_stor
         let(:commit_message) { 'test' }
         let(:branch) { "#{Time.current.to_i}_dashboard_new_branch" }
         let(:file_name) { 'custom_dashboard.yml' }
-
-        [::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH].each do |dashboard_template|
-          context "dashboard template #{dashboard_template}" do
-            let(:dashboard) { dashboard_template }
-            let(:params) do
-              {
-                dashboard: dashboard,
-                file_name: file_name,
-                commit_message: commit_message,
-                branch: branch
-              }
-            end
-
-            it 'delegates commit creation to Files::CreateService', :aggregate_failures do
-              dashboard_attrs = {
-                commit_message: commit_message,
-                branch_name: branch,
-                start_branch: 'master',
-                encoding: 'text',
-                file_path: '.gitlab/dashboards/custom_dashboard.yml',
-                file_content: File.read(dashboard)
-              }
-
-              service_instance = instance_double(::Files::CreateService)
-              expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
-              expect(service_instance).to receive(:execute).and_return(status: :success)
-
-              described_class.new(project, user, params).execute
-            end
-          end
+        let(:file_content_hash) { YAML.safe_load(File.read(dashboard)) }
+        let(:params) do
+          {
+            dashboard: dashboard,
+            file_name: file_name,
+            commit_message: commit_message,
+            branch: branch
+          }
         end
+
+        it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, [STAGES::CommonMetricsInserter, STAGES::ProjectMetricsInserter, STAGES::Sorter]
+        it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH, [STAGES::CommonMetricsInserter, STAGES::Sorter]
       end
     end
   end

spec/services/metrics/dashboard/clone_dashboard_service_spec.rb

@@ -5,6 +5,8 @@ require 'spec_helper'
 describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_store_caching do
   include MetricsDashboardHelpers
 
+  STAGES = ::Gitlab::Metrics::Dashboard::Stages
+
   set(:user) { create(:user) }
   set(:project) { create(:project, :repository) }
   set(:environment) { create(:environment, project: project) }
@@ -16,6 +18,7 @@ describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_stor
     let(:branch) { "dashboard_new_branch" }
     let(:dashboard) { 'config/prometheus/common_metrics.yml' }
     let(:file_name) { 'custom_dashboard.yml' }
+    let(:file_content_hash) { YAML.safe_load(File.read(dashboard)) }
     let(:params) do
       {
         dashboard: dashboard,
@@ -25,17 +28,6 @@ describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_stor
       }
     end
 
-    let(:dashboard_attrs) do
-      {
-        commit_message: commit_message,
-        branch_name: branch,
-        start_branch: project.default_branch,
-        encoding: 'text',
-        file_path: ".gitlab/dashboards/#{file_name}",
-        file_content: File.read(dashboard)
-      }
-    end
-
     context 'user does not have push right to repository' do
       it_behaves_like 'misconfigured dashboard service response', :forbidden, %q(You can't commit to this project)
     end
@@ -72,11 +64,12 @@ describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_stor
             start_branch: project.default_branch,
             encoding: 'text',
             file_path: ".gitlab/dashboards/custom_dashboard.yml",
-            file_content: File.read(dashboard)
+            file_content:  file_content_hash.to_yaml
           }
         end
 
         it 'strips target file name to safe value', :aggregate_failures do
+          allow(::Gitlab::Metrics::Dashboard::Processor).to receive(:new).and_return(double(process: file_content_hash))
           service_instance = instance_double(::Files::CreateService)
           expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
           expect(service_instance).to receive(:execute).and_return(status: :success)
@@ -86,14 +79,12 @@ describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_stor
       end
 
       context 'valid parameters' do
-        it 'delegates commit creation to Files::CreateService', :aggregate_failures do
-          service_instance = instance_double(::Files::CreateService)
-          expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
-          expect(service_instance).to receive(:execute).and_return(status: :success)
-
-          service_call
+        before do
+          allow(::Gitlab::Metrics::Dashboard::Processor).to receive(:new).and_return(double(process: file_content_hash))
         end
 
+        it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, [STAGES::CommonMetricsInserter, STAGES::ProjectMetricsInserter, STAGES::Sorter]
+
         context 'selected branch already exists' do
           let(:branch) { 'existing_branch' }
 

spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb

@@ -50,3 +50,38 @@ RSpec.shared_examples 'raises error for users with insufficient permissions' do
     it_behaves_like 'misconfigured dashboard service response', :unauthorized
   end
 end
+
+RSpec.shared_examples 'valid dashboard cloning process' do |dashboard_template, sequence|
+  context "dashboard template: #{dashboard_template}" do
+    let(:dashboard) { dashboard_template }
+    let(:dashboard_attrs) do
+      {
+        commit_message: commit_message,
+        branch_name: branch,
+        start_branch: project.default_branch,
+        encoding: 'text',
+        file_path: ".gitlab/dashboards/#{file_name}",
+        file_content: file_content_hash.to_yaml
+      }
+    end
+
+    it 'delegates commit creation to Files::CreateService', :aggregate_failures do
+      service_instance = instance_double(::Files::CreateService)
+      expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
+      expect(service_instance).to receive(:execute).and_return(status: :success)
+
+      service_call
+    end
+
+    context 'user has defined custom metrics' do
+      it 'uses external service to includes them into new file content', :aggregate_failures do
+        service_instance = double(::Gitlab::Metrics::Dashboard::Processor)
+        expect(::Gitlab::Metrics::Dashboard::Processor).to receive(:new).with(project, file_content_hash, sequence, {}).and_return(service_instance)
+        expect(service_instance).to receive(:process).and_return(file_content_hash)
+        expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(double(execute: { status: :success }))
+
+        service_call
+      end
+    end
+  end
+end