Fix DORA deployment frequency in VSA

This change fixes the license mismatch between Value Stream Analytics
and DORA metrics features. Premium and Ultimate Value Stream Analytics
users can also use the DORA based deploymeny frequency metric.

Changelog: changed

ee/app/services/dora/aggregate_metrics_service.rb

@@ -7,6 +7,20 @@ module Dora
     DEFAULT_INTERVAL = Dora::DailyMetrics::INTERVAL_DAILY
 
     def execute
+      if error = validate_container
+        return error
+      end
+
+      return authorization_error unless authorized?
+
+      execute_without_authorization
+    end
+
+    def execute_without_authorization
+      if error = validate_container
+        return error
+      end
+
       if error = validate
         return error
       end
@@ -21,6 +35,22 @@ module Dora
 
     private
 
+    def authorized?
+      return false unless project_container? || group_container?
+
+      can?(current_user, :read_dora4_analytics, container)
+    end
+
+    def authorization_error
+      error(_('You do not have permission to access dora metrics.'), :unauthorized)
+    end
+
+    def validate_container
+      unless project_container? || group_container?
+        error(_('Container must be a project or a group.'), :bad_request)
+      end
+    end
+
     def validate
       unless (end_date - start_date) <= MAX_RANGE
         return error(_("Date range must be shorter than %{max_range} days.") % { max_range: MAX_RANGE },
@@ -31,10 +61,6 @@ module Dora
         return error(_('The start date must be ealier than the end date.'), :bad_request)
       end
 
-      unless project? || group?
-        return error(_('Container must be a project or a group.'), :bad_request)
-      end
-
       if group_project_ids.present? && !group?
         return error(_('The group_project_ids parameter is only allowed for a group'), :bad_request)
       end
@@ -54,10 +80,6 @@ module Dora
                      :bad_request)
       end
 
-      unless can?(current_user, :read_dora4_analytics, container)
-        return error(_('You do not have permission to access dora metrics.'), :unauthorized)
-      end
-
       nil
     end
 

ee/lib/ee/gitlab/cycle_analytics/summary/deploy.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module CycleAnalytics
+      module Summary
+        module Deploy
+          extend ::Gitlab::Utils::Override
+
+          private
+
+          override :deployments_count
+          def deployments_count
+            if project.licensed_feature_available?(:cycle_analytics_for_projects)
+              deployment_count_via_dora_api
+            else
+              super
+            end
+          end
+
+          def deployment_count_via_dora_api
+            result = Dora::AggregateMetricsService.new(
+              container: project,
+              current_user: options[:current_user],
+              params: dora_aggregate_metrics_params
+            ).execute_without_authorization
+
+            result[:status] == :success ? (result[:data] || 0) : 0
+          end
+
+          def dora_aggregate_metrics_params
+            {
+              start_date: options[:from].to_date,
+              end_date: (options[:to] || Date.today).to_date,
+              interval: 'all',
+              environment_tier: 'production',
+              metric: 'deployment_frequency'
+            }
+          end
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/analytics/cycle_analytics/summary/group/deploy.rb

@@ -42,7 +42,7 @@ module Gitlab
                 container: group,
                 current_user: options[:current_user],
                 params: dora_aggregate_metrics_params
-              ).execute
+              ).execute_without_authorization
 
               result[:status] == :success ? (result[:data] || 0) : 0
             end

ee/spec/lib/gitlab/analytics/cycle_analytics/summary/group/stage_summary_spec.rb

@@ -148,8 +148,6 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::Summary::Group::StageSummary d
         subject { described_class.new(group, options: { from: two_days_ago, current_user: user }).data }
 
         before do
-          stub_licensed_features(dora4_analytics: true)
-
           travel_to(five_days_ago) do
             create_deployment(project: project, finished_at: Time.current)
             create_deployment(project: project_2, finished_at: Time.current)
@@ -235,8 +233,6 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::Summary::Group::StageSummary d
         end
 
         before do
-          stub_licensed_features(dora4_analytics: true)
-
           travel_to(five_days_ago) do
             create_deployment(finished_at: Time.current, project: project)
           end

ee/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::CycleAnalytics::StageSummary do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user, :admin) }
+
+  let(:options) { { from: 1.day.ago } }
+  let(:args) { { options: options, current_user: user } }
+
+  before(:all) do
+    project.add_maintainer(user)
+  end
+
+  let(:stage_summary) { described_class.new(project, **args).data }
+
+  it_behaves_like 'deployment metrics examples'
+
+  it 'does not use the DORA API' do
+    expect(Dora::AggregateMetricsService).not_to receive(:new).and_call_original
+
+    stage_summary
+  end
+
+  context 'when cycle_analytics_for_projects feature is available' do
+    before do
+      stub_licensed_features(cycle_analytics_for_projects: true)
+    end
+
+    it_behaves_like 'deployment metrics examples'
+
+    it 'uses the DORA API' do
+      expect(Dora::AggregateMetricsService).to receive(:new).and_call_original
+
+      stage_summary
+    end
+  end
+end

ee/spec/services/dora/aggregate_metrics_service_spec.rb

@@ -5,15 +5,15 @@ require 'spec_helper'
 RSpec.describe Dora::AggregateMetricsService do
   let(:service) { described_class.new(container: container, current_user: user, params: params) }
 
-  describe '#execute' do
-    subject { service.execute }
-
   around do |example|
     freeze_time do
       example.run
     end
   end
 
+  describe '#execute' do
+    subject { service.execute }
+
     shared_examples_for 'request failure' do
       it 'returns error' do
         expect(subject[:status]).to eq(:error)
@@ -227,4 +227,34 @@ RSpec.describe Dora::AggregateMetricsService do
       end
     end
   end
+
+  describe '#execute_without_authorization' do
+    context 'runs the service without authorization' do
+      subject { service.execute_without_authorization }
+
+      context 'when passign a non-ultimate group' do
+        let_it_be(:group) { create(:group) }
+        let_it_be(:project) { create(:project, group: group) }
+        let_it_be(:production) { create(:environment, :production, project: project) }
+        let_it_be(:maintainer) { create(:user) }
+
+        let(:container) { group }
+        let(:user) { maintainer }
+        let(:params) { { environment_tier: 'production', interval: 'all', metric: 'deployment_frequency' } }
+
+        before do
+          group.add_maintainer(maintainer)
+
+          create(:dora_daily_metrics, deployment_frequency: 2, environment: production)
+
+          stub_licensed_features(dora4_analytics: false)
+        end
+
+        it 'loads the deployment frequency metrics' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:data]).to eq(2)
+        end
+      end
+    end
+  end
 end

lib/gitlab/cycle_analytics/summary/base.rb

@@ -16,6 +16,10 @@ module Gitlab
         def value
           raise NotImplementedError, "Expected #{self.name} to implement value"
         end
+
+        private
+
+        attr_reader :project, :options
       end
     end
   end

lib/gitlab/cycle_analytics/summary/deploy.rb

@@ -24,3 +24,5 @@ module Gitlab
     end
   end
 end
+
+Gitlab::CycleAnalytics::Summary::Deploy.prepend_mod_with('Gitlab::CycleAnalytics::Summary::Deploy')

spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::CycleAnalytics::StageSummary do
-  let(:project) { create(:project, :repository) }
+  let_it_be(:project) { create(:project, :repository) }
+
   let(:options) { { from: 1.day.ago } }
   let(:args) { { options: options, current_user: user } }
   let(:user) { create(:user, :admin) }
@@ -62,6 +63,8 @@ RSpec.describe Gitlab::CycleAnalytics::StageSummary do
   end
 
   describe "#commits" do
+    let!(:project) { create(:project, :repository) }
+
     subject { stage_summary.second }
 
     context 'when from date is given' do
@@ -132,115 +135,5 @@ RSpec.describe Gitlab::CycleAnalytics::StageSummary do
     end
   end
 
-  describe "#deploys" do
-    subject { stage_summary.third }
-
-    context 'when from date is given' do
-      before do
-        Timecop.freeze(5.days.ago) { create(:deployment, :success, project: project) }
-        Timecop.freeze(5.days.from_now) { create(:deployment, :success, project: project) }
-      end
-
-      it "finds the number of deploys made created after the 'from date'" do
-        expect(subject[:value]).to eq('1')
-      end
-
-      it 'returns the localized title' do
-        Gitlab::I18n.with_locale(:ru) do
-          expect(subject[:title]).to eq(n_('Deploy', 'Deploys', 1))
-        end
-      end
-    end
-
-    it "doesn't find commits from other projects" do
-      Timecop.freeze(5.days.from_now) do
-        create(:deployment, :success, project: create(:project, :repository))
-      end
-
-      expect(subject[:value]).to eq('-')
-    end
-
-    context 'when `to` parameter is given' do
-      before do
-        Timecop.freeze(5.days.ago) { create(:deployment, :success, project: project) }
-        Timecop.freeze(5.days.from_now) { create(:deployment, :success, project: project) }
-      end
-
-      it "doesn't find any record" do
-        options[:to] = Time.now
-
-        expect(subject[:value]).to eq('-')
-      end
-
-      it "finds records created between `from` and `to` range" do
-        options[:from] = 10.days.ago
-        options[:to] = 10.days.from_now
-
-        expect(subject[:value]).to eq('2')
-      end
-    end
-  end
-
-  describe '#deployment_frequency' do
-    subject { stage_summary.fourth[:value] }
-
-    it 'includes the unit: `per day`' do
-      expect(stage_summary.fourth[:unit]).to eq _('per day')
-    end
-
-    before do
-      Timecop.freeze(5.days.ago) { create(:deployment, :success, project: project) }
-    end
-
-    it 'returns 0.0 when there were deploys but the frequency was too low' do
-      options[:from] = 30.days.ago
-
-      # 1 deployment over 30 days
-      # frequency of 0.03, rounded off to 0.0
-      expect(subject).to eq('0')
-    end
-
-    it 'returns `-` when there were no deploys' do
-      options[:from] = 4.days.ago
-
-      # 0 deployment in the last 4 days
-      expect(subject).to eq('-')
-    end
-
-    context 'when `to` is nil' do
-      it 'includes range until now' do
-        options[:from] = 6.days.ago
-        options[:to] = nil
-
-        # 1 deployment over 7 days
-        expect(subject).to eq('0.1')
-      end
-    end
-
-    context 'when `to` is given' do
-      before do
-        Timecop.freeze(5.days.from_now) { create(:deployment, :success, project: project, finished_at: Time.zone.now) }
-      end
-
-      it 'finds records created between `from` and `to` range' do
-        options[:from] = 10.days.ago
-        options[:to] = 10.days.from_now
-
-        # 2 deployments over 20 days
-        expect(subject).to eq('0.1')
-      end
-
-      context 'when `from` and `to` are within a day' do
-        it 'returns the number of deployments made on that day' do
-          freeze_time do
-            create(:deployment, :success, project: project, finished_at: Time.zone.now)
-            options[:from] = Time.zone.now.at_beginning_of_day
-            options[:to] = Time.zone.now.at_end_of_day
-
-            expect(subject).to eq('1')
-          end
-        end
-      end
-    end
-  end
+  it_behaves_like 'deployment metrics examples'
 end

spec/support/shared_examples/lib/gitlab/cycle_analytics/deployment_metrics.rb

+# frozen_string_literal: true
+
+shared_examples 'deployment metrics examples' do
+  def create_deployment(args)
+    project = args[:project]
+    environment = project.environments.production.first || create(:environment, :production, project: project)
+    create(:deployment, :success, args.merge(environment: environment))
+
+    # this is needed for the dora_deployment_frequency_in_vsa feature flag so we have aggregated data
+    ::Dora::DailyMetrics::RefreshWorker.new.perform(environment.id, Time.current.to_date.to_s) if Gitlab.ee?
+  end
+
+  describe "#deploys" do
+    subject { stage_summary.third }
+
+    context 'when from date is given' do
+      before do
+        travel_to(5.days.ago) { create_deployment(project: project) }
+        create_deployment(project: project)
+      end
+
+      it "finds the number of deploys made created after the 'from date'" do
+        expect(subject[:value]).to eq('1')
+      end
+
+      it 'returns the localized title' do
+        Gitlab::I18n.with_locale(:ru) do
+          expect(subject[:title]).to eq(n_('Deploy', 'Deploys', 1))
+        end
+      end
+    end
+
+    it "doesn't find commits from other projects" do
+      travel_to(5.days.from_now) do
+        create_deployment(project: create(:project, :repository))
+      end
+
+      expect(subject[:value]).to eq('-')
+    end
+
+    context 'when `to` parameter is given' do
+      before do
+        travel_to(5.days.ago) { create_deployment(project: project) }
+        travel_to(5.days.from_now) { create_deployment(project: project) }
+      end
+
+      it "doesn't find any record" do
+        options[:to] = Time.now
+
+        expect(subject[:value]).to eq('-')
+      end
+
+      it "finds records created between `from` and `to` range" do
+        options[:from] = 10.days.ago
+        options[:to] = 10.days.from_now
+
+        expect(subject[:value]).to eq('2')
+      end
+    end
+  end
+
+  describe '#deployment_frequency' do
+    subject { stage_summary.fourth[:value] }
+
+    it 'includes the unit: `per day`' do
+      expect(stage_summary.fourth[:unit]).to eq _('per day')
+    end
+
+    before do
+      travel_to(5.days.ago) { create_deployment(project: project) }
+    end
+
+    it 'returns 0.0 when there were deploys but the frequency was too low' do
+      options[:from] = 30.days.ago
+
+      # 1 deployment over 30 days
+      # frequency of 0.03, rounded off to 0.0
+      expect(subject).to eq('0')
+    end
+
+    it 'returns `-` when there were no deploys' do
+      options[:from] = 4.days.ago
+
+      # 0 deployment in the last 4 days
+      expect(subject).to eq('-')
+    end
+
+    context 'when `to` is nil' do
+      it 'includes range until now' do
+        options[:from] = 6.days.ago
+        options[:to] = nil
+
+        # 1 deployment over 7 days
+        expect(subject).to eq('0.1')
+      end
+    end
+
+    context 'when `to` is given' do
+      before do
+        travel_to(5.days.from_now) { create_deployment(project: project, finished_at: Time.zone.now) }
+      end
+
+      it 'finds records created between `from` and `to` range' do
+        options[:from] = 10.days.ago
+        options[:to] = 10.days.from_now
+
+        # 2 deployments over 20 days
+        expect(subject).to eq('0.1')
+      end
+
+      context 'when `from` and `to` are within a day' do
+        it 'returns the number of deployments made on that day' do
+          freeze_time do
+            create_deployment(project: project, finished_at: Time.current)
+            options[:from] = Time.current.yesterday.beginning_of_day
+            options[:to] = Time.current.end_of_day
+
+            expect(subject).to eq('0.5')
+          end
+        end
+      end
+    end
+  end
+end