From 15431054e239646a845082a9f03cc579dacfc3b9 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Tue, 13 Nov 2018 13:49:38 +0900
Subject: Add spec for deployment metrics

---
 app/models/deployment.rb                           |  6 ++--
 app/serializers/environment_status_entity.rb       |  6 +++-
 spec/serializers/environment_status_entity_spec.rb | 36 ++++++++++++++++++++++
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 83434276995..811e623b7f7 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -160,18 +160,18 @@ class Deployment < ActiveRecord::Base
   end
 
   def has_metrics?
-    prometheus_adapter&.can_query?
+    prometheus_adapter&.can_query? && success?
   end
 
   def metrics
-    return {} unless has_metrics? && success?
+    return {} unless has_metrics?
 
     metrics = prometheus_adapter.query(:deployment, self)
     metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
 
   def additional_metrics
-    return {} unless has_metrics? && success?
+    return {} unless has_metrics?
 
     metrics = prometheus_adapter.query(:additional_metrics_deployment, self)
     metrics&.merge(deployment_time: finished_at.to_i) || {}
diff --git a/app/serializers/environment_status_entity.rb b/app/serializers/environment_status_entity.rb
index f6321b9e520..5171da22c9e 100644
--- a/app/serializers/environment_status_entity.rb
+++ b/app/serializers/environment_status_entity.rb
@@ -11,7 +11,7 @@ class EnvironmentStatusEntity < Grape::Entity
     project_environment_path(es.project, es.environment)
   end
 
-  expose :metrics_url, if: ->(*) { can_read_environment? && deployment.has_metrics? } do |es|
+  expose :metrics_url, if: ->(*) { can_read_deployment? && deployment.has_metrics? } do |es|
     metrics_project_environment_deployment_path(es.project, es.environment, es.deployment)
   end
 
@@ -61,6 +61,10 @@ class EnvironmentStatusEntity < Grape::Entity
     can?(current_user, :read_environment, environment)
   end
 
+  def can_read_deployment?
+    can?(current_user, :read_deployment, deployment)
+  end
+
   def can_stop_environment?
     can?(current_user, :stop_environment, environment)
   end
diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb
index 58e2e627410..8a6a38fe5f8 100644
--- a/spec/serializers/environment_status_entity_spec.rb
+++ b/spec/serializers/environment_status_entity_spec.rb
@@ -40,4 +40,40 @@ describe EnvironmentStatusEntity do
 
     it { is_expected.to include(:stop_url) }
   end
+
+  context 'when deployment has metrics' do
+    let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) }
+
+    let(:simple_metrics) do
+      {
+        success: true,
+        metrics: {},
+        last_update: 42
+      }
+    end
+
+    before do
+      project.add_maintainer(user)
+      allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter)
+      allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics)
+      allow(entity).to receive(:deployment).and_return(deployment)
+    end
+
+    context 'when deployment succeeded' do
+      let(:deployment)    { create(:deployment, :succeed, :review_app) }
+
+      it 'returns metrics url' do
+        expect(subject[:metrics_url])
+          .to eq("/#{project.namespace.name}/#{project.name}/environments/#{environment.id}/deployments/#{deployment.iid}/metrics")
+      end
+    end
+
+    context 'when deployment is running' do
+      let(:deployment)    { create(:deployment, :running, :review_app) }
+
+      it 'does not return metrics url' do
+        expect(subject[:metrics_url]).to be_nil
+      end
+    end
+  end
 end
-- 
cgit v1.2.1