Merge branch '337256-fix-vsa-deployment-frequency' into 'master'

[RUN-AS-IF-FOSS] Fix DORA deployment frequency in VSA

See merge request gitlab-org/gitlab!69654

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) }
 
+  around do |example|
+    freeze_time do
+      example.run
+    end
+  end
+
   describe '#execute' do
     subject { service.execute }
 
-    around do |example|
-      freeze_time do
-        example.run
-      end
-    end
-
     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