Keep presentation logic in one place and remove unecessary arguments.

+ fix tests

6 files changed, 22 insertions, 17 deletions

diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index f4751dc5334..216cec751e3 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -106,8 +106,7 @@ class Deployment < ActiveRecord::Base
   def metrics
     return {} unless has_metrics?
 
-    metrics = project.monitoring_service.deployment_metrics(self)
-    metrics&.merge(deployment_time: created_at.to_i) || {}
+    project.monitoring_service.deployment_metrics(self)
   end
 
   private
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb
index 23e19bedf3f..ec72cb6856d 100644
--- a/app/models/project_services/prometheus_service.rb
+++ b/app/models/project_services/prometheus_service.rb
@@ -63,12 +63,13 @@ class PrometheusService < MonitoringService
     { success: false, result: err }
   end
 
-  def environment_metrics(environment, **args)
+  def environment_metrics(environment)
     with_reactive_cache(Gitlab::Prometheus::Queries::EnvironmentQuery.name, environment.id, &:itself)
   end
 
   def deployment_metrics(deployment)
-    with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.id, &:itself)
+    metrics = with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.id, &:itself)
+    metrics&.merge(deployment_time: created_at.to_i) || {}
   end
 
   # Cache metrics for specific environment
diff --git a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb
index c69cd180547..e739005ba83 100644
--- a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb
+++ b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb
@@ -1,28 +1,32 @@
 require 'spec_helper'
 
 describe Gitlab::Prometheus::Queries::DeploymentQuery, lib: true do
-  let(:environment) { create(:environment) }
+  let(:environment) { create(:environment, slug: 'environment-slug') }
   let(:deployment) { create(:deployment, environment: environment) }
 
   let(:client) { double('prometheus_client') }
   subject { described_class.new(client) }
 
+  around do |example|
+    Timecop.freeze { example.run }
+  end
+
   it 'sends appropriate queries to prometheus' do
     start_time = (deployment.created_at - 30.minutes).to_f
 
     stop_time = (deployment.created_at + 30.minutes).to_f
-    expect(client).to receive(:query_range).with('avg(container_memory_usage_bytes{container_name!="POD",environment="environment1"}) / 2^20',
+    expect(client).to receive(:query_range).with('avg(container_memory_usage_bytes{container_name!="POD",environment="environment-slug"}) / 2^20',
                                                  start: start_time, stop: stop_time)
-    expect(client).to receive(:query).with('avg(avg_over_time(container_memory_usage_bytes{container_name!="POD",environment="environment1"}[30m]))',
+    expect(client).to receive(:query).with('avg(avg_over_time(container_memory_usage_bytes{container_name!="POD",environment="environment-slug"}[30m]))',
                                            time: deployment.created_at.to_f)
-    expect(client).to receive(:query).with('avg(avg_over_time(container_memory_usage_bytes{container_name!="POD",environment="environment1"}[30m]))',
+    expect(client).to receive(:query).with('avg(avg_over_time(container_memory_usage_bytes{container_name!="POD",environment="environment-slug"}[30m]))',
                                            time: stop_time)
 
-    expect(client).to receive(:query_range).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment1"}[2m])) * 100',
+    expect(client).to receive(:query_range).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment-slug"}[2m])) * 100',
                                                  start: start_time, stop: stop_time)
-    expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment1"}[30m])) * 100',
+    expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment-slug"}[30m])) * 100',
                                            time: deployment.created_at.to_f)
-    expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment1"}[30m])) * 100',
+    expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment-slug"}[30m])) * 100',
                                            time: stop_time)
 
     expect(subject.query(deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil,
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index 212fcd884a8..4bda7d4314a 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -52,7 +52,7 @@ describe Deployment, models: true do
   describe '#metrics' do
     let(:deployment) { create(:deployment) }
 
-    subject { deployment.metrics(1.hour) }
+    subject { deployment.metrics }
 
     context 'metrics are disabled' do
       it { is_expected.to eq({}) }
@@ -63,16 +63,17 @@ describe Deployment, models: true do
         {
           success: true,
           metrics: {},
-          last_update: 42
+          last_update: 42,
+          deployment_time: 1494408956
         }
       end
 
       before do
-        allow(deployment.project).to receive_message_chain(:monitoring_service, :metrics)
+        allow(deployment.project).to receive_message_chain(:monitoring_service, :deployment_metrics)
                                        .with(any_args).and_return(simple_metrics)
       end
 
-      it { is_expected.to eq(simple_metrics.merge(deployment_time: deployment.created_at.utc.to_i)) }
+      it { is_expected.to eq(simple_metrics) }
     end
   end
 
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index edc1c204014..12519de8636 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -393,7 +393,7 @@ describe Environment, models: true do
 
       it 'returns the metrics from the deployment service' do
         expect(project.monitoring_service)
-          .to receive(:metrics).with(environment)
+          .to receive(:environment_metrics).with(environment)
           .and_return(:fake_metrics)
 
         is_expected.to eq(:fake_metrics)
diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb
index 739850a836e..1f9d3c07b51 100644
--- a/spec/models/project_services/prometheus_service_spec.rb
+++ b/spec/models/project_services/prometheus_service_spec.rb
@@ -82,7 +82,7 @@ describe PrometheusService, models: true, caching: true do
       end
 
       it 'returns reactive data' do
-        is_expected.to eq(prometheus_data)
+        is_expected.to eq(prometheus_data.merge(deployment_time: deployment.created_at.to_i))
       end
     end
   end