Merge branch 'environments-dashboard-limits-and-optimization' into 'master'

Environments Dashboard Limits and Optimization

See merge request gitlab-org/gitlab!19775

app/models/environment.rb

@@ -12,7 +12,7 @@ class Environment < ApplicationRecord
   has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment'
   has_one :last_deployable, through: :last_deployment, source: 'deployable', source_type: 'CommitStatus'
   has_one :last_pipeline, through: :last_deployable, source: 'pipeline'
-  has_one :last_visible_deployment, -> { visible.distinct_on_environment }, class_name: 'Deployment'
+  has_one :last_visible_deployment, -> { visible.distinct_on_environment }, inverse_of: :environment, class_name: 'Deployment'
   has_one :last_visible_deployable, through: :last_visible_deployment, source: 'deployable', source_type: 'CommitStatus'
   has_one :last_visible_pipeline, through: :last_visible_deployable, source: 'pipeline'
 
@@ -66,6 +66,9 @@ class Environment < ApplicationRecord
   scope :for_project, -> (project) { where(project_id: project) }
   scope :with_deployment, -> (sha) { where('EXISTS (?)', Deployment.select(1).where('deployments.environment_id = environments.id').where(sha: sha)) }
   scope :unfoldered, -> { where(environment_type: nil) }
+  scope :with_rank, -> do
+    select('environments.*, rank() OVER (PARTITION BY project_id ORDER BY id DESC)')
+  end
 
   state_machine :state, initial: :available do
     event :start do

app/models/project.rb

@@ -287,7 +287,7 @@ class Project < ApplicationRecord
   has_many :variables, class_name: 'Ci::Variable'
   has_many :triggers, class_name: 'Ci::Trigger'
   has_many :environments
-  has_many :unfoldered_environments, -> { unfoldered.available }, class_name: 'Environment'
+  has_many :environments_for_dashboard, -> { from(with_rank.unfoldered.available, :environments).where('rank <= 3') }, class_name: 'Environment'
   has_many :deployments
   has_many :pipeline_schedules, class_name: 'Ci::PipelineSchedule'
   has_many :project_deploy_tokens

ee/app/serializers/dashboard_environment_entity.rb

@@ -12,11 +12,15 @@ class DashboardEnvironmentEntity < Grape::Entity
   expose :external_url
 
   expose :last_visible_deployment, as: :last_deployment, expose_nil: false do |environment|
-    DeploymentEntity.represent(environment.last_visible_deployment, options.merge(request: request_with_project))
+    DeploymentEntity.represent(environment.last_visible_deployment,
+                               options.merge(request: request_with_project,
+                                             except: unnecessary_deployment_fields))
   end
 
   expose :last_visible_pipeline, as: :last_pipeline, expose_nil: false do |environment|
-    PipelineDetailsEntity.represent(environment.last_visible_pipeline, options.merge(request: request_with_project))
+    PipelineDetailsEntity.represent(environment.last_visible_pipeline,
+                                    options.merge(request: request_with_project,
+                                                  only: required_pipeline_fields))
   end
 
   private
@@ -29,4 +33,17 @@ class DashboardEnvironmentEntity < Grape::Entity
       project: environment.project
     )
   end
+
+  def unnecessary_deployment_fields
+    [:deployed_by, :manual_actions, :scheduled_actions, :cluster]
+  end
+
+  def required_pipeline_fields
+    [
+      :id,
+      { details: :detailed_status },
+      { triggered_by: { details: :detailed_status } },
+      { triggered: { details: :detailed_status } }
+    ]
+  end
 end

ee/app/serializers/dashboard_environments_project_entity.rb

@@ -13,5 +13,5 @@ class DashboardEnvironmentsProjectEntity < Grape::Entity
   end
 
   expose :namespace, using: API::Entities::NamespaceBasic
-  expose :unfoldered_environments, as: :environments, using: DashboardEnvironmentEntity
+  expose :environments_for_dashboard, as: :environments, using: DashboardEnvironmentEntity
 end

ee/app/services/dashboard/environments/list_service.rb

@@ -3,6 +3,8 @@
 module Dashboard
   module Environments
     class ListService
+      MAX_NUM_PROJECTS = 7
+
       def initialize(user)
         @user = user
       end
@@ -15,11 +17,35 @@ module Dashboard
 
       attr_reader :user
 
+      # rubocop: disable CodeReuse/ActiveRecord
       def load_projects(user)
-        ::Dashboard::Operations::ProjectsService
+        projects = ::Dashboard::Operations::ProjectsService
           .new(user)
-          .execute(user.ops_dashboard_projects)
+          .execute(user.ops_dashboard_projects, limit: MAX_NUM_PROJECTS)
+
+        ActiveRecord::Associations::Preloader.new.preload(projects, [
+          :route,
+          environments_for_dashboard: [
+            last_visible_pipeline: [
+              :user,
+              project: [:route, :group, :project_feature, namespace: :route]
+            ],
+            last_visible_deployment: [
+              deployable: [
+                :metadata,
+                :pipeline,
+                project: [:project_feature, :group, :route, namespace: :route]
+              ],
+              project: [:route, namespace: :route]
+            ],
+            project: [:project_feature, :group, namespace: :route]
+          ],
+          namespace: [:route, :owner]
+        ])
+
+        projects
       end
+      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

ee/app/services/dashboard/operations/projects_service.rb

@@ -7,11 +7,12 @@ module Dashboard
         @user = user
       end
 
-      def execute(project_ids, include_unavailable: false)
+      def execute(project_ids, include_unavailable: false, limit: nil)
         return [] unless License.feature_available?(:operations_dashboard)
 
         projects = find_projects(user, project_ids).to_a
         projects = available_projects(projects) unless include_unavailable
+        projects = limit ? projects.first(limit) : projects
 
         projects
       end

ee/spec/controllers/operations_controller_spec.rb

@@ -5,6 +5,9 @@ require 'spec_helper'
 describe OperationsController do
   include Rails.application.routes.url_helpers
 
+  PUBLIC = Gitlab::VisibilityLevel::PUBLIC
+  PRIVATE = Gitlab::VisibilityLevel::PRIVATE
+
   let(:user) { create(:user) }
 
   shared_examples 'unlicensed' do |http_method, action|
@@ -165,6 +168,22 @@ describe OperationsController do
         expect(expected_project['last_alert']['id']).to eq(last_firing_alert.id)
       end
 
+      it "returns as many projects as are in the user's dashboard" do
+        projects = Array.new(8).map do
+          project = create(:project)
+          project.add_developer(user)
+          project
+        end
+        user.update!(ops_dashboard_projects: projects)
+
+        get :list
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('dashboard/operations/list', dir: 'ee')
+
+        expect(json_response['projects'].size).to eq(8)
+      end
+
       it 'returns a list of added projects regardless of the environments_dashboard feature flag' do
         stub_feature_flags(environments_dashboard: { enabled: false, thing: user })
 
@@ -435,6 +454,105 @@ describe OperationsController do
           expect(last_deployment_json['id']).to eq(deployment.id)
         end
 
+        it 'returns a maximum of seven projects' do
+          projects = Array.new(8).map do
+            project = create(:project)
+            project.add_developer(user)
+            project
+          end
+          user.update!(ops_dashboard_projects: projects)
+
+          get :environments_list
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+          expect(json_response['projects'].count).to eq(7)
+        end
+
+        it 'does not return a project for which the operations dashboard feature is unavailable' do
+          stub_application_setting(check_namespace_plan: true)
+          namespace = create(:namespace, visibility_level: PRIVATE)
+          unavailable_project = create(:project, namespace: namespace, visibility_level: PRIVATE)
+          unavailable_project.add_developer(user)
+          user.update!(ops_dashboard_projects: [unavailable_project])
+
+          get :environments_list
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+          expect(json_response['projects'].count).to eq(0)
+        end
+
+        it 'returns seven projects when some projects do not have the dashboard feature available' do
+          stub_application_setting(check_namespace_plan: true)
+
+          public_namespace = create(:namespace, visibility_level: PUBLIC)
+          public_projects = Array.new(7).map do
+            project = create(:project, namespace: public_namespace, visibility_level: PUBLIC)
+            project.add_developer(user)
+            project
+          end
+
+          private_namespace = create(:namespace, visibility_level: PRIVATE)
+          private_project = create(:project, namespace: private_namespace, visibility_level: PRIVATE)
+          private_project.add_developer(user)
+
+          all_projects = [private_project] + public_projects
+          user.update!(ops_dashboard_projects: all_projects)
+
+          get :environments_list
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+          expect(json_response['projects'].count).to eq(7)
+
+          actual_ids = json_response['projects'].map { |p| p['id'].to_i }
+          expected_ids = public_projects.map(&:id)
+
+          expect(actual_ids).to contain_exactly(*expected_ids)
+        end
+
+        it 'returns a maximum of three environments for a project' do
+          create(:environment, project: project)
+          create(:environment, project: project)
+          create(:environment, project: project)
+          create(:environment, project: project)
+
+          get :environments_list
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+
+          project_json = json_response['projects'].first
+
+          expect(project_json['environments'].count).to eq(3)
+        end
+
+        it 'returns a maximum of three environments for multiple projects' do
+          project_b = create(:project)
+          project_b.add_developer(user)
+          create(:environment, project: project)
+          create(:environment, project: project)
+          create(:environment, project: project)
+          create(:environment, project: project)
+          create(:environment, project: project_b)
+          create(:environment, project: project_b)
+          create(:environment, project: project_b)
+          create(:environment, project: project_b)
+          user.update!(ops_dashboard_projects: [project, project_b])
+
+          get :environments_list
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+
+          project_json = json_response['projects'].find { |p| p['id'] == project.id }
+          project_b_json = json_response['projects'].find { |p| p['id'] == project_b.id }
+
+          expect(project_json['environments'].count).to eq(3)
+          expect(project_b_json['environments'].count).to eq(3)
+        end
+
         context 'with a pipeline' do
           let(:project) { create(:project, :repository) }
           let(:commit) { project.commit }
@@ -460,6 +578,8 @@ describe OperationsController do
             last_pipeline_json = environment_json['last_pipeline']
 
             expect(last_pipeline_json['id']).to eq(pipeline.id)
+            expect(last_pipeline_json['triggered']).to eq([])
+            expect(last_pipeline_json['triggered_by']).to be_nil
           end
 
           it 'returns the last pipeline details' do
@@ -475,8 +595,73 @@ describe OperationsController do
             project_json = json_response['projects'].first
             environment_json = project_json['environments'].first
             last_pipeline_json = environment_json['last_pipeline']
+            expected_details_path = project_pipeline_path(project, pipeline)
 
             expect(last_pipeline_json.dig('details', 'status', 'group')).to eq('canceled')
+            expect(last_pipeline_json.dig('details', 'status', 'tooltip')).to eq('canceled')
+            expect(last_pipeline_json.dig('details', 'status', 'details_path')).to eq(expected_details_path)
+          end
+
+          it 'returns an upstream pipeline' do
+            project_b = create(:project, :repository)
+            project_b.add_developer(user)
+            commit_b = project_b.commit
+            pipeline_b = create(:ci_pipeline, project: project_b, user: user, sha: commit_b.sha)
+            ci_build_b = create(:ci_build, project: project_b, pipeline: pipeline_b)
+            pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
+            ci_build = create(:ci_build, project: project, pipeline: pipeline)
+            create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
+            create(:ci_sources_pipeline, project: project, pipeline: pipeline, source_project: project_b, source_pipeline: pipeline_b, source_job: ci_build_b)
+
+            get :environments_list
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+
+            project_json = json_response['projects'].first
+            environment_json = project_json['environments'].first
+            last_pipeline_json = environment_json['last_pipeline']
+            triggered_by_pipeline_json = last_pipeline_json['triggered_by']
+
+            expected_details_path = project_pipeline_path(project_b, pipeline_b)
+
+            expect(last_pipeline_json['id']).to eq(pipeline.id)
+            expect(triggered_by_pipeline_json['id']).to eq(pipeline_b.id)
+            expect(triggered_by_pipeline_json.dig('details', 'status', 'details_path')).to eq(expected_details_path)
+            expect(triggered_by_pipeline_json.dig('project', 'full_name')).to eq(project_b.full_name)
+          end
+
+          it 'returns a downstream pipeline' do
+            project_b = create(:project, :repository)
+            project_b.add_developer(user)
+            commit_b = project_b.commit
+            pipeline_b = create(:ci_pipeline, :failed, project: project_b, user: user, sha: commit_b.sha)
+            create(:ci_build, :failed, project: project_b, pipeline: pipeline_b)
+            pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
+            ci_build = create(:ci_build, project: project, pipeline: pipeline)
+            create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
+            create(:ci_sources_pipeline, project: project_b, pipeline: pipeline_b, source_project: project, source_pipeline: pipeline, source_job: ci_build)
+
+            get :environments_list
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
+
+            project_json = json_response['projects'].first
+            environment_json = project_json['environments'].first
+            last_pipeline_json = environment_json['last_pipeline']
+
+            expect(last_pipeline_json['triggered'].count).to eq(1)
+
+            triggered_pipeline_json = last_pipeline_json['triggered'].first
+
+            expected_details_path = project_pipeline_path(project_b, pipeline_b)
+
+            expect(last_pipeline_json['id']).to eq(pipeline.id)
+            expect(triggered_pipeline_json['id']).to eq(pipeline_b.id)
+            expect(triggered_pipeline_json.dig('details', 'status', 'details_path')).to eq(expected_details_path)
+            expect(triggered_pipeline_json.dig('details', 'status', 'group')).to eq('failed')
+            expect(triggered_pipeline_json.dig('project', 'full_name')).to eq(project_b.full_name)
           end
         end
       end

ee/spec/services/dashboard/environments/list_service_spec.rb

@@ -4,21 +4,54 @@ require 'spec_helper'
 
 describe Dashboard::Environments::ListService do
   describe '#execute' do
+    def setup
+      user = create(:user)
+      project = create(:project)
+      project.add_developer(user)
+      user.update!(ops_dashboard_projects: [project])
+
+      [user, project]
+    end
+
     before do
       stub_licensed_features(operations_dashboard: true)
     end
 
     it 'returns a list of projects' do
-      user = create(:user)
-      project = create(:project)
-      project.add_developer(user)
-      user.update!(ops_dashboard_projects: [project])
+      user, project = setup
 
       projects_with_environments = described_class.new(user).execute
 
       expect(projects_with_environments).to eq([project])
     end
 
+    it 'preloads only relevant ci_builds' do
+      user, project = setup
+
+      ci_build_a = create(:ci_build, project: project)
+      ci_build_b = create(:ci_build, project: project)
+      ci_build_c = create(:ci_build, project: project)
+
+      environment_a = create(:environment, project: project)
+      environment_b = create(:environment, project: project)
+
+      create(:deployment, :success, project: project, environment: environment_a, deployable: ci_build_a)
+      create(:deployment, :success, project: project, environment: environment_a, deployable: ci_build_b)
+      create(:deployment, :success, project: project, environment: environment_b, deployable: ci_build_c)
+
+      expect(CommitStatus).to receive(:instantiate)
+        .with(a_hash_including("id" => ci_build_b.id), anything)
+        .at_least(:once)
+        .and_call_original
+
+      expect(CommitStatus).to receive(:instantiate)
+        .with(a_hash_including("id" => ci_build_c.id), anything)
+        .at_least(:once)
+        .and_call_original
+
+      described_class.new(user).execute
+    end
+
     context 'when unlicensed' do
       before do
         stub_licensed_features(operations_dashboard: false)

spec/lib/gitlab/import_export/all_models.yml

@@ -340,7 +340,7 @@ project:
 - triggers
 - pipeline_schedules
 - environments
-- unfoldered_environments
+- environments_for_dashboard
 - deployments
 - project_feature
 - auto_devops