Add unit tests and fix broken endpoint

6 files changed, 183 insertions, 17 deletions

diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb
index b10f0c7b7db..da35d8d37f7 100644
--- a/app/controllers/projects/environments_controller.rb
+++ b/app/controllers/projects/environments_controller.rb
@@ -158,11 +158,11 @@ class Projects::EnvironmentsController < Projects::ApplicationController
   end
 
   def metrics_dashboard
-    access_denied! unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project)
+    render_403 and return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project)
 
     respond_to do |format|
       format.json do
-        dashboard = MetricsDashboardService.new.get_dashboard
+        dashboard = MetricsDashboardService.new(@project).get_dashboard
 
         render json: dashboard, status: :ok
       end
diff --git a/app/services/metrics_dashboard_processing_service.rb b/app/services/metrics_dashboard_processing_service.rb
index 2bfeccc2644..6dc8db00934 100644
--- a/app/services/metrics_dashboard_processing_service.rb
+++ b/app/services/metrics_dashboard_processing_service.rb
@@ -9,40 +9,68 @@ class MetricsDashboardProcessingService
   end
 
   def process
-    insert_persisted_metrics!
     insert_metric_ids!
+    sort_groups!
+    sort_panels!
+    insert_project_metrics!
     @dashboard.to_json
   end
 
   private
 
+  # ------- Processing Steps -----------
+
+  # For each metric in the dashboard config, attempts to find a corresponding
+  # database record. If found, includes the record's id in the dashboard config.
+  def insert_metric_ids!
+    for_metrics do |metric|
+      metric_record = common_metrics.find { |m| m.identifier == metric[:id] }
+      metric[:metric_id] = metric_record.id if metric_record
+    end
+  end
+
   # Inserts project-specific metrics into the dashboard config.
   # If there are no project-specific metrics, this will have no effect.
-  def insert_persisted_metrics!
-    @project.prometheus_metrics.each do |persisted_metric|
-      group = find_or_create_group(@dashboard[:panel_groups], persisted_metric)
-      panel = find_or_create_panel(group[:panels], persisted_metric)
-      find_or_create_metric(panel[:metrics], persisted_metric)
+  def insert_project_metrics!
+    project_metrics.each do |project_metric|
+      group = find_or_create_group(@dashboard[:panel_groups], project_metric)
+      panel = find_or_create_panel(group[:panels], project_metric)
+      find_or_create_metric(panel[:metrics], project_metric)
     end
   end
 
-  # For each metric in the dashboard config, attempts to find a corresponding
-  # persisted record. If found, includes the record id in the config.
-  def insert_metric_ids!
+  # Sorts the groups in the dashboard by the :priority key
+  def sort_groups!
+    @dashboard[:panel_groups] = @dashboard[:panel_groups].sort_by { |group| group[:priority] }
+  end
+
+  # Sorts the panels in the dashboard by the :weight key
+  def sort_panels!
     @dashboard[:panel_groups].each do |group|
-      group[:panels].each do |panel|
-        panel[:metrics].each do |metric|
-          metric_record = common_metrics.find {|m| m.identifier == metric[:id] }
-          metric[:metric_id] = metric_record.id if metric_record
-        end
-      end
+      group[:panels] = group[:panels].sort_by { |panel| panel[:weight] }
     end
   end
 
+  # ------- Processing Helpers -----------
+
+  def project_metrics
+    @project.prometheus_metrics
+  end
+
   def common_metrics
     @common_metrics ||= ::PrometheusMetric.common
   end
 
+  def for_metrics
+    @dashboard[:panel_groups].each do |panel_group|
+      panel_group[:panels].each do |panel|
+        panel[:metrics].each do |metric|
+          yield metric
+        end
+      end
+    end
+  end
+
   def find_or_create_group(panel_groups, metric)
     target_group = panel_groups.find { |group| group[:group] == metric.group_title }
 
diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb
index 75158f2e8e0..02441385da0 100644
--- a/spec/controllers/projects/environments_controller_spec.rb
+++ b/spec/controllers/projects/environments_controller_spec.rb
@@ -461,6 +461,30 @@ describe Projects::EnvironmentsController do
     end
   end
 
+  describe 'metrics_dashboard' do
+    context 'when prometheus endpoint is disabled' do
+      before do
+        stub_feature_flags(environment_metrics_use_prometheus_endpoint: false)
+      end
+
+      it 'responds with status code 403' do
+        get :metrics_dashboard, params: environment_params(format: :json)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'when prometheus endpoint is enabled' do
+      it 'returns a json representation of the environment dashboard' do
+        get :metrics_dashboard, params: environment_params(format: :json)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to include('dashboard', 'order', 'panel_groups')
+        expect(json_response['panel_groups']).to all( include('group', 'priority', 'panels') )
+      end
+    end
+  end
+
   describe 'GET #search' do
     before do
       create(:environment, name: 'staging', project: project)
diff --git a/spec/fixtures/services/metrics_dashboard_processing_service.yml b/spec/fixtures/services/metrics_dashboard_processing_service.yml
new file mode 100644
index 00000000000..ebfe06da6db
--- /dev/null
+++ b/spec/fixtures/services/metrics_dashboard_processing_service.yml
@@ -0,0 +1,36 @@
+dashboard: 'Test Dashboard'
+order: 1
+panel_groups:
+- group: Group A
+  priority: 10
+  panels:
+  - title: "Super Chart A1"
+    type: "area-chart"
+    y_label: "y_label"
+    weight: 2
+    metrics:
+    - id: metric_a1
+      query_range: 'query'
+      unit: unit
+      label: Legend Label
+  - title: "Super Chart A2"
+    type: "area-chart"
+    y_label: "y_label"
+    weight: 1
+    metrics:
+    - id: metric_a2
+      query_range: 'query'
+      label: Legend Label
+      unit: unit
+- group: Group B
+  priority: 1
+  panels:
+  - title: "Super Chart B"
+    type: "area-chart"
+    y_label: "y_label"
+    weight: 1
+    metrics:
+    - id: metric_b
+      query_range: 'query'
+      unit: unit
+      label: Legend Label
diff --git a/spec/services/metrics_dashboard_processing_service_spec.rb b/spec/services/metrics_dashboard_processing_service_spec.rb
new file mode 100644
index 00000000000..658ff1dab49
--- /dev/null
+++ b/spec/services/metrics_dashboard_processing_service_spec.rb
@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+describe MetricsDashboardProcessingService do
+  let(:project)   { build(:project) }
+  let(:dashboard_yml) { YAML.load_file('spec/fixtures/services/metrics_dashboard_processing_service.yml') }
+
+  describe 'process' do
+    let(:dashboard) { JSON.parse(described_class.new(dashboard_yml, project).process, symbolize_names: true) }
+
+    context 'when dashboard config corresponds to common metrics' do
+      let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') }
+
+      it 'inserts metric ids into the config' do
+        target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' }
+
+        expect(target_metric).to include(:metric_id)
+      end
+    end
+
+    context 'when the project has associated metrics' do
+      let!(:project_metric) { create(:prometheus_metric, project: project) }
+
+      it 'includes project-specific metrics' do
+
+        project_metric_details = {
+          query_range: project_metric.query,
+          unit: project_metric.unit,
+          label: project_metric.legend,
+          metric_id: project_metric.id,
+        }
+
+        expect(all_metrics).to include project_metric_details
+      end
+
+      it 'includes project metrics at the end of the config' do
+        expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1', nil]
+        actual_metrics_order = all_metrics.map { |m| m[:id] }
+
+        expect(actual_metrics_order).to eq expected_metrics_order
+      end
+    end
+
+    it 'orders groups by priority and panels by weight' do
+      expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1']
+      actual_metrics_order = all_metrics.map { |m| m[:id] }
+
+      expect(actual_metrics_order).to eq expected_metrics_order
+    end
+  end
+
+  def all_metrics
+    dashboard[:panel_groups].map do |group|
+      group[:panels].map { |panel| panel[:metrics] }
+    end.flatten
+  end
+end
diff --git a/spec/services/metrics_dashboard_service_spec.rb b/spec/services/metrics_dashboard_service_spec.rb
new file mode 100644
index 00000000000..9b7994fa34f
--- /dev/null
+++ b/spec/services/metrics_dashboard_service_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe MetricsDashboardService, :use_clean_rails_memory_store_caching do
+  let(:project)  { build(:project) }
+
+  describe 'get_dashboard' do
+    it 'returns a json representation of the environment dashboard' do
+      dashboard = described_class.new(project).get_dashboard
+      json = JSON.parse(dashboard, symbolize_names: true)
+
+      expect(json).to include(:dashboard, :order, :panel_groups)
+      expect(json[:panel_groups]).to all( include(:group, :priority, :panels) )
+    end
+
+    it 'caches the dashboard for subsequent calls' do
+      expect(YAML).to receive(:load_file).once.and_call_original
+
+      described_class.new(project).get_dashboard
+      described_class.new(project).get_dashboard
+    end
+  end
+end