Merge branch 'sy-dashboard-endpoint' into 'master'

Refactor metrics dashboard endpoints into a module

See merge request gitlab-org/gitlab!17675

app/controllers/concerns/metrics_dashboard.rb

+# frozen_string_literal: true
+
+# Provides an action which fetches a metrics dashboard according
+# to the parameters specified by the controller.
+module MetricsDashboard
+  extend ActiveSupport::Concern
+
+  def metrics_dashboard
+    result = dashboard_finder.find(
+      project_for_dashboard,
+      current_user,
+      metrics_dashboard_params
+    )
+
+    if include_all_dashboards?
+      result[:all_dashboards] = dashboard_finder.find_all_paths(project_for_dashboard)
+    end
+
+    respond_to do |format|
+      if result[:status] == :success
+        format.json { render dashboard_success_response(result) }
+      else
+        format.json { render dashboard_error_response(result) }
+      end
+    end
+  end
+
+  private
+
+  # Override in class to provide arguments to the finder.
+  def metrics_dashboard_params
+    {}
+  end
+
+  # Override in class if response requires complete list of
+  # dashboards in addition to requested dashboard body.
+  def include_all_dashboards?
+    false
+  end
+
+  def dashboard_finder
+    ::Gitlab::Metrics::Dashboard::Finder
+  end
+
+  # Project is not defined for group and admin level clusters.
+  def project_for_dashboard
+    defined?(project) ? project : nil
+  end
+
+  def dashboard_success_response(result)
+    {
+      status: :ok,
+      json: result.slice(:all_dashboards, :dashboard, :status)
+    }
+  end
+
+  def dashboard_error_response(result)
+    {
+      status: result[:http_status],
+      json: result.slice(:all_dashboards, :message, :status)
+    }
+  end
+end

app/controllers/projects/environments_controller.rb

 # frozen_string_literal: true
 
 class Projects::EnvironmentsController < Projects::ApplicationController
+  include MetricsDashboard
+
   layout 'project'
   before_action :authorize_read_environment!
   before_action :authorize_create_environment!, only: [:new, :create]
@@ -158,42 +160,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController
     end
   end
 
-  def metrics_dashboard
-    if params[:embedded]
-      result = dashboard_finder.find(
-        project,
-        current_user,
-        environment: environment,
-        dashboard_path: params[:dashboard],
-        **dashboard_params.to_h.symbolize_keys
-      )
-    else
-      result = dashboard_finder.find(
-        project,
-        current_user,
-        environment: environment,
-        dashboard_path: params[:dashboard]
-      )
-
-      result[:all_dashboards] = dashboard_finder.find_all_paths(project)
-    end
-
-    respond_to do |format|
-      if result[:status] == :success
-        format.json do
-          render status: :ok, json: result.slice(:all_dashboards, :dashboard, :status)
-        end
-      else
-        format.json do
-          render(
-            status: result[:http_status],
-            json: result.slice(:all_dashboards, :message, :status)
-          )
-        end
-      end
-    end
-  end
-
   def search
     respond_to do |format|
       format.json do
@@ -231,12 +197,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController
     params.require([:start, :end])
   end
 
-  def dashboard_params
-    params.permit(:embedded, :group, :title, :y_label)
+  def metrics_dashboard_params
+    params
+      .permit(:embedded, :group, :title, :y_label)
+      .to_h.symbolize_keys
+      .merge(dashboard_path: params[:dashboard], environment: environment)
   end
 
-  def dashboard_finder
-    Gitlab::Metrics::Dashboard::Finder
+  def include_all_dashboards?
+    !params[:embedded]
   end
 
   def search_environment_names

ee/app/controllers/ee/clusters/clusters_controller.rb

@@ -3,6 +3,7 @@
 module EE
   module Clusters
     module ClustersController
+      include MetricsDashboard
       extend ActiveSupport::Concern
 
       prepended do
@@ -50,26 +51,6 @@ module EE
         end
       end
 
-      def metrics_dashboard
-        project_for_dashboard = defined?(project) ? project : nil # Project is not defined for group and admin level clusters
-        dashboard = ::Gitlab::Metrics::Dashboard::Finder.find(project_for_dashboard, current_user, metrics_dashboard_params)
-
-        respond_to do |format|
-          if dashboard[:status] == :success
-            format.json do
-              render status: :ok, json: dashboard.slice(:dashboard, :status)
-            end
-          else
-            format.json do
-              render(
-                status: dashboard[:http_status],
-                json: dashboard.slice(:message, :status)
-              )
-            end
-          end
-        end
-      end
-
       private
 
       def prometheus_adapter

ee/spec/support/shared_examples/controllers/cluster_metrics_examples.rb

@@ -188,40 +188,14 @@ shared_examples 'cluster metrics' do
     end
   end
 
-  shared_examples_for 'correctly formatted response' do |status_code|
+  shared_examples_for 'the default dashboard' do
     it 'returns a json object with the correct keys' do
       get :metrics_dashboard, params: metrics_params, format: :json
 
-      found_keys = json_response.keys - ['all_dashboards']
-
-      expect(response).to have_gitlab_http_status(status_code)
-      expect(found_keys).to contain_exactly(*expected_keys)
-    end
-  end
-
-  shared_examples_for '200 response' do
-    let(:expected_keys) { %w(dashboard status) }
-
-    it_behaves_like 'correctly formatted response', :ok
-  end
-
-  shared_context 'error response' do |status_code|
-    let(:expected_keys) { %w(message status) }
-
-    it_behaves like 'correctly formatted response', status_code
-  end
-
-  shared_examples_for 'includes all dashboards' do
-    it 'includes info for all findable dashboards' do
-      expect(json_response).to have_key('all_dashboards')
-      expect(json_response['all_dashboards']).to be_an_instance_of(Array)
-      expect(json_response['all_dashboards']).to all( include('path', 'default', 'display_name') )
-    end
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response.keys).to contain_exactly('dashboard', 'status')
     end
 
-  shared_examples_for 'the default dashboard' do
-    it_behaves_like '200 response'
-
     it 'is the default dashboard' do
       get :metrics_dashboard, params: metrics_params, format: :json
 

lib/gitlab/metrics/dashboard/finder.rb

@@ -20,13 +20,17 @@ module Gitlab
           # @param options - dashboard_path [String] Path at which the
           #         dashboard can be found. Nil values will
           #         default to the system dashboard.
-          # @param options - group [String] Title of the group
+          # @param options - group [String, Group] Title of the group
           #         to which a panel might belong. Used by
-          #         embedded dashboards.
+          #         embedded dashboards. If cluster dashboard,
+          #         refers to the Group corresponding to the cluster.
           # @param options - title [String] Title of the panel.
           #         Used by embedded dashboards.
           # @param options - y_label [String] Y-Axis label of
           #         a panel. Used by embedded dashboards.
+          # @param options - cluster [Cluster]
+          # @param options - cluster_type [Symbol] The level of
+          #         cluster, one of [:admin, :project, :group]
           # @return [Hash]
           def find(project, user, options = {})
             service_for(options)

spec/controllers/concerns/metrics_dashboard_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MetricsDashboard do
+  describe 'GET #metrics_dashboard' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:environment) { create(:environment, project: project) }
+
+    before do
+      sign_in(user)
+      project.add_maintainer(user)
+    end
+
+    controller(::ApplicationController) do
+      include MetricsDashboard # rubocop:disable RSpec/DescribedClass
+    end
+
+    let(:json_response) do
+      routes.draw { get "metrics_dashboard" => "anonymous#metrics_dashboard" }
+      response = get :metrics_dashboard, format: :json
+
+      JSON.parse(response.parsed_body)
+    end
+
+    context 'when no parameters are provided' do
+      it 'returns an error json_response' do
+        expect(json_response['status']).to eq('error')
+      end
+    end
+
+    context 'when params are provided' do
+      before do
+        allow(controller).to receive(:project).and_return(project)
+        allow(controller)
+          .to receive(:metrics_dashboard_params)
+          .and_return(environment: environment)
+      end
+
+      it 'returns the specified dashboard' do
+        expect(json_response['dashboard']['dashboard']).to eq('Environment metrics')
+        expect(json_response).not_to have_key('all_dashboards')
+      end
+
+      context 'when parameters are provided and the list of all dashboards is required' do
+        before do
+          allow(controller).to receive(:include_all_dashboards?).and_return(true)
+        end
+
+        it 'returns a dashboard in addition to the list of dashboards' do
+          expect(json_response['dashboard']['dashboard']).to eq('Environment metrics')
+          expect(json_response).to have_key('all_dashboards')
+        end
+      end
+    end
+  end
+end