Merge branch 'group-level-dora-metrics-rest-api' into 'master'

Group-level DORA metrics API

See merge request gitlab-org/gitlab!56059

ee/app/services/dora/aggregate_metrics_service.rb

@@ -31,8 +31,8 @@ module Dora
         return error(_('The start date must be ealier than the end date.'), :bad_request)
       end
 
-      unless project?
-        return error(_('Container must be a project.'), :bad_request)
+      unless project? || group?
+        return error(_('Container must be a project or a group.'), :bad_request)
       end
 
       unless ::Dora::DailyMetrics::AVAILABLE_INTERVALS.include?(interval)
@@ -58,13 +58,32 @@ module Dora
     end
 
     def environments
-      Environment.for_project(container).for_tier(environment_tier)
+      Environment.for_project(target_projects).for_tier(environment_tier)
+    end
+
+    def target_projects
+      if project?
+        [container]
+      elsif group?
+        # The actor definitely has read permission in all subsequent projects of the group by the following reasons:
+        # - DORA metrics can be read by reporter (or above) at project-level.
+        # - With `read_dora4_analytics` permission check, we make sure that the
+        #   user is at-least reporter role at group-level.
+        # - In the subsequent projects, the assigned role at the group-level
+        #   can't be lowered. For example, if the user is reporter at group-level,
+        #   the user can be developer in subsequent projects, but can't be guest.
+        container.all_projects
+      end
     end
 
     def project?
       container.is_a?(Project)
     end
 
+    def group?
+      container.is_a?(Group)
+    end
+
     def start_date
       params[:start_date] || 3.months.ago.to_date
     end

ee/lib/api/dora/metrics.rb

@@ -5,6 +5,30 @@ module API
     class Metrics < ::API::Base
       feature_category :continuous_delivery
 
+      helpers do
+        params :dora_metrics_params do
+          requires :metric, type: String, desc: 'The metric type.'
+          optional :start_date, type: Date, desc: 'Date range to start from.'
+          optional :end_date, type: Date, desc: 'Date range to end at.'
+          optional :interval, type: String, desc: "The bucketing interval."
+          optional :environment_tier, type: String, desc: "The tier of the environment."
+        end
+
+        def fetch!(container)
+          not_found! unless ::Feature.enabled?(:dora_daily_metrics, container, default_enabled: :yaml)
+
+          result = ::Dora::AggregateMetricsService
+            .new(container: container, current_user: current_user, params: declared_params(include_missing: false))
+            .execute
+
+          if result[:status] == :success
+            present result[:data]
+          else
+            render_api_error!(result[:message], result[:http_status])
+          end
+        end
+      end
+
       params do
         requires :id, type: String, desc: 'The ID of the project'
       end
@@ -12,11 +36,7 @@ module API
         namespace ':id/dora/metrics' do
           desc 'Fetch the project-level DORA metrics'
           params do
-            requires :metric, type: String, desc: 'The metric type.'
-            optional :start_date, type: Date, desc: 'Date range to start from.'
-            optional :end_date, type: Date, desc: 'Date range to end at.'
-            optional :interval, type: String, desc: "The bucketing interval."
-            optional :environment_tier, type: String, desc: "The tier of the environment."
+            use :dora_metrics_params
           end
           get do
             fetch!(user_project)
@@ -24,18 +44,17 @@ module API
         end
       end
 
-      helpers do
-        def fetch!(container)
-          not_found! unless ::Feature.enabled?(:dora_daily_metrics, container, default_enabled: :yaml)
-
-          result = ::Dora::AggregateMetricsService
-            .new(container: container, current_user: current_user, params: declared_params(include_missing: false))
-            .execute
-
-          if result[:status] == :success
-            present result[:data]
-          else
-            render_api_error!(result[:message], result[:http_status])
+      params do
+        requires :id, type: String, desc: 'The ID of the group'
+      end
+      resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+        namespace ':id/dora/metrics' do
+          desc 'Fetch the group-level DORA metrics'
+          params do
+            use :dora_metrics_params
+          end
+          get do
+            fetch!(user_group)
           end
         end
       end

ee/spec/requests/api/dora/metrics_spec.rb

@@ -62,4 +62,65 @@ RSpec.describe API::Dora::Metrics do
       end
     end
   end
+
+  describe 'GET /groups/:id/dora/metrics' do
+    subject { get api(url, user), params: params }
+
+    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_it_be(:guest) { create(:user) }
+    let(:url) { "/groups/#{group.id}/dora/metrics" }
+    let(:params) { { metric: :deployment_frequency } }
+    let(:user) { maintainer }
+
+    around do |example|
+      freeze_time do
+        example.run
+      end
+    end
+
+    before_all do
+      group.add_maintainer(maintainer)
+      group.add_guest(guest)
+      create(:dora_daily_metrics, deployment_frequency: 1, environment: production, date: 1.day.ago.to_date)
+      create(:dora_daily_metrics, deployment_frequency: 2, environment: production, date: Time.current.to_date)
+    end
+
+    before do
+      stub_licensed_features(dora4_analytics: true)
+    end
+
+    it 'returns data' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response).to eq([{ 1.day.ago.to_date.to_s => 1 },
+                                   { Time.current.to_date.to_s => 2 }])
+    end
+
+    context 'when user is guest' do
+      let(:user) { guest }
+
+      it 'returns authorization error' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(json_response['message']).to eq('You do not have permission to access dora metrics.')
+      end
+    end
+
+    context 'when dora_daily_metrics feature flag is disabled' do
+      before do
+        stub_feature_flags(dora_daily_metrics: false)
+      end
+
+      it 'returns not found' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
 end

ee/spec/services/dora/aggregate_metrics_service_spec.rb

@@ -22,6 +22,71 @@ RSpec.describe Dora::AggregateMetricsService do
       end
     end
 
+    shared_examples_for 'correct validations' do
+      context 'when data range is too wide' do
+        let(:extra_params) { { start_date: 1.year.ago.to_date } }
+
+        it_behaves_like 'request failure' do
+          let(:message) { "Date range must be shorter than #{described_class::MAX_RANGE} days." }
+          let(:http_status) { :bad_request }
+        end
+      end
+
+      context 'when start date is later than end date' do
+        let(:extra_params) { { end_date: 1.year.ago.to_date } }
+
+        it_behaves_like 'request failure' do
+          let(:message) { 'The start date must be ealier than the end date.' }
+          let(:http_status) { :bad_request }
+        end
+      end
+
+      context 'when interval is invalid' do
+        let(:extra_params) { { interval: 'unknown' } }
+
+        it_behaves_like 'request failure' do
+          let(:message) { "The interval must be one of #{::Dora::DailyMetrics::AVAILABLE_INTERVALS.join(',')}." }
+          let(:http_status) { :bad_request }
+        end
+      end
+
+      context 'when metric is invalid' do
+        let(:extra_params) { { metric: 'unknown' } }
+
+        it_behaves_like 'request failure' do
+          let(:message) { "The metric must be one of #{::Dora::DailyMetrics::AVAILABLE_METRICS.join(',')}." }
+          let(:http_status) { :bad_request }
+        end
+      end
+
+      context 'when params is empty' do
+        let(:params) { {} }
+
+        it_behaves_like 'request failure' do
+          let(:message) { "The metric must be one of #{::Dora::DailyMetrics::AVAILABLE_METRICS.join(',')}." }
+          let(:http_status) { :bad_request }
+        end
+      end
+
+      context 'when environment tier is invalid' do
+        let(:extra_params) { { environment_tier: 'unknown' } }
+
+        it_behaves_like 'request failure' do
+          let(:message) { "The environment tier must be one of #{Environment.tiers.keys.join(',')}." }
+          let(:http_status) { :bad_request }
+        end
+      end
+
+      context 'when guest user' do
+        let(:user) { guest }
+
+        it_behaves_like 'request failure' do
+          let(:message) { 'You do not have permission to access dora metrics.' }
+          let(:http_status) { :unauthorized }
+        end
+      end
+    end
+
     context 'when container is project' do
       let_it_be(:project) { create(:project) }
       let_it_be(:production) { create(:environment, :production, project: project) }
@@ -45,6 +110,8 @@ RSpec.describe Dora::AggregateMetricsService do
         stub_licensed_features(dora4_analytics: true)
       end
 
+      it_behaves_like 'correct validations'
+
       it 'returns the aggregated data' do
         expect(subject[:status]).to eq(:success)
         expect(subject[:data]).to eq([{ Time.current.to_date.to_s => 2 }])
@@ -76,81 +143,66 @@ RSpec.describe Dora::AggregateMetricsService do
           expect(subject[:data]).to eq([{ Time.current.to_date.to_s => 1 }])
         end
       end
+    end
 
-      context 'when data range is too wide' do
-        let(:extra_params) { { start_date: 1.year.ago.to_date } }
-
-        it_behaves_like 'request failure' do
-          let(:message) { "Date range must be shorter than #{described_class::MAX_RANGE} days." }
-          let(:http_status) { :bad_request }
-        end
-      end
-
-      context 'when start date is later than end date' do
-        let(:extra_params) { { end_date: 1.year.ago.to_date } }
-
-        it_behaves_like 'request failure' do
-          let(:message) { 'The start date must be ealier than the end date.' }
-          let(:http_status) { :bad_request }
-        end
-      end
+    context 'when container is a group' do
+      let_it_be(:group) { create(:group) }
+      let_it_be(:project_1) { create(:project, group: group) }
+      let_it_be(:project_2) { create(:project, group: group) }
+      let_it_be(:production_1) { create(:environment, :production, project: project_1) }
+      let_it_be(:production_2) { create(:environment, :production, project: project_2) }
+      let_it_be(:maintainer) { create(:user) }
+      let_it_be(:guest) { create(:user) }
+      let(:container) { group }
+      let(:user) { maintainer }
+      let(:params) { { metric: 'deployment_frequency' }.merge(extra_params) }
+      let(:extra_params) { {} }
 
-      context 'when interval is invalid' do
-        let(:extra_params) { { interval: 'unknown' } }
+      before_all do
+        group.add_maintainer(maintainer)
+        group.add_guest(guest)
 
-        it_behaves_like 'request failure' do
-          let(:message) { "The interval must be one of #{::Dora::DailyMetrics::AVAILABLE_INTERVALS.join(',')}." }
-          let(:http_status) { :bad_request }
-        end
+        create(:dora_daily_metrics, deployment_frequency: 2, environment: production_1)
+        create(:dora_daily_metrics, deployment_frequency: 1, environment: production_2)
       end
 
-      context 'when metric is invalid' do
-        let(:extra_params) { { metric: 'unknown' } }
-
-        it_behaves_like 'request failure' do
-          let(:message) { "The metric must be one of #{::Dora::DailyMetrics::AVAILABLE_METRICS.join(',')}." }
-          let(:http_status) { :bad_request }
-        end
+      before do
+        stub_licensed_features(dora4_analytics: true)
       end
 
-      context 'when params is empty' do
-        let(:params) { {} }
+      it_behaves_like 'correct validations'
 
-        it_behaves_like 'request failure' do
-          let(:message) { "The metric must be one of #{::Dora::DailyMetrics::AVAILABLE_METRICS.join(',')}." }
-          let(:http_status) { :bad_request }
-        end
+      it 'returns the aggregated data' do
+        expect(subject[:status]).to eq(:success)
+        expect(subject[:data]).to eq([{ Time.current.to_date.to_s => 3 }])
       end
 
-      context 'when environment tier is invalid' do
-        let(:extra_params) { { environment_tier: 'unknown' } }
+      context 'when interval is monthly' do
+        let(:extra_params) { { interval: Dora::DailyMetrics::INTERVAL_MONTHLY } }
 
-        it_behaves_like 'request failure' do
-          let(:message) { "The environment tier must be one of #{Environment.tiers.keys.join(',')}." }
-          let(:http_status) { :bad_request }
+        it 'returns the aggregated data' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:data]).to eq([{ Time.current.beginning_of_month.to_date.to_s => 3 }])
         end
       end
 
-      context 'when guest user' do
-        let(:user) { guest }
+      context 'when interval is all' do
+        let(:extra_params) { { interval: Dora::DailyMetrics::INTERVAL_ALL } }
 
-        it_behaves_like 'request failure' do
-          let(:message) { 'You do not have permission to access dora metrics.' }
-          let(:http_status) { :unauthorized }
+        it 'returns the aggregated data' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:data]).to eq(3)
         end
       end
     end
 
-    context 'when container is group' do
-      let_it_be(:group) { create(:group) }
-      let_it_be(:maintainer) { create(:user) }
-      let_it_be(:guest) { create(:user) }
-      let(:container) { group }
-      let(:user) { maintainer }
-      let(:params) { { metric: 'deployment_frequency' } }
+    context 'when container is nil' do
+      let(:container) { nil }
+      let(:user) { nil }
+      let(:params) { {} }
 
       it_behaves_like 'request failure' do
-        let(:message) { 'Container must be a project.' }
+        let(:message) { 'Container must be a project or a group.' }
         let(:http_status) { :bad_request }
       end
     end

locale/gitlab.pot

@@ -8023,7 +8023,7 @@ msgstr ""
 msgid "Container does not exist"
 msgstr ""
 
-msgid "Container must be a project."
+msgid "Container must be a project or a group."
 msgstr ""
 
 msgid "Container registry images"