Merge branch 'optimize-environment-dasshboard-query' into 'master'

Fix last_deployment Eager Load for Environment Dashboard

See merge request gitlab-org/gitlab!18935

app/models/deployment.rb

@@ -75,6 +75,11 @@ class Deployment < ApplicationRecord
     find(ids)
   end
 
+  def self.distinct_on_environment
+    order('environment_id, deployments.id DESC')
+      .select('DISTINCT ON (environment_id) deployments.*')
+  end
+
   def self.find_successful_deployment!(iid)
     success.find_by!(iid: iid)
   end

app/models/environment.rb

@@ -10,6 +10,7 @@ class Environment < ApplicationRecord
   has_many :successful_deployments, -> { success }, class_name: 'Deployment'
 
   has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment'
+  has_one :last_visible_deployment, -> { visible.distinct_on_environment }, class_name: 'Deployment'
 
   before_validation :nullify_external_url
   before_validation :generate_slug, if: ->(env) { env.slug.blank? }

ee/app/models/environment_folder.rb

@@ -15,7 +15,7 @@ class EnvironmentFolder
 
     environments_by_id = environments
       .id_in(folder_data.map { |(env_id, _)| env_id })
-      .includes(:project, last_deployment: [:project, deployable: :user])
+      .includes(:project, last_visible_deployment: [:project, deployable: :user])
       .index_by(&:id)
 
     folders = folder_data.map do |(environment_id, count)|

ee/app/serializers/dashboard_environment_entity.rb

@@ -11,15 +11,15 @@ class DashboardEnvironmentEntity < Grape::Entity
 
   expose :external_url
 
-  expose :last_deployment, expose_nil: false do |environment|
-    DeploymentEntity.represent(environment.last_deployment, options.merge(request: new_request))
+  expose :last_visible_deployment, as: :last_deployment, expose_nil: false do |environment|
+    DeploymentEntity.represent(environment.last_visible_deployment, options.merge(request: request_with_project))
   end
 
   private
 
   alias_method :environment, :object
 
-  def new_request
+  def request_with_project
     EntityRequest.new(
       current_user: request.current_user,
       project: environment.project

ee/spec/controllers/operations_controller_spec.rb

@@ -485,6 +485,22 @@ describe OperationsController do
           expect(deployable_json['id']).to eq(ci_build.id)
           expect(deployable_json['build_path']).to eq(project_job_path(project, ci_build))
         end
+
+        it 'returns a failed deployment' do
+          environment = create(:environment, project: project)
+          deployment = create(:deployment, :failed, project: project, environment: environment)
+
+          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_deployment_json = environment_json['last_deployment']
+
+          expect(last_deployment_json['id']).to eq(deployment.id)
+        end
       end
     end
   end

ee/spec/models/environment_folder_spec.rb

@@ -58,5 +58,30 @@ describe EnvironmentFolder do
 
       expect(environment_folder.last_environment).to eq(environment)
     end
+
+    it 'preloads only relevant ci_builds' do
+      project = create(:project)
+
+      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)
+        .and_call_original
+
+      expect(CommitStatus).to receive(:instantiate)
+        .with(a_hash_including("id" => ci_build_c.id), anything)
+        .and_call_original
+
+      described_class.find_for_projects([project])
+    end
   end
 end

spec/models/environment_spec.rb

@@ -515,6 +515,48 @@ describe Environment, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '#last_visible_deployment' do
+    subject { environment.last_visible_deployment }
+
+    before do
+      allow_any_instance_of(Deployment).to receive(:create_ref)
+    end
+
+    context 'when there is an old deployment record' do
+      let!(:previous_deployment) { create(:deployment, :success, environment: environment) }
+
+      context 'when there is a deployment record with created status' do
+        let!(:deployment) { create(:deployment, environment: environment) }
+
+        it { is_expected.to eq(previous_deployment) }
+      end
+
+      context 'when there is a deployment record with running status' do
+        let!(:deployment) { create(:deployment, :running, environment: environment) }
+
+        it { is_expected.to eq(deployment) }
+      end
+
+      context 'when there is a deployment record with success status' do
+        let!(:deployment) { create(:deployment, :success, environment: environment) }
+
+        it { is_expected.to eq(deployment) }
+      end
+
+      context 'when there is a deployment record with failed status' do
+        let!(:deployment) { create(:deployment, :failed, environment: environment) }
+
+        it { is_expected.to eq(deployment) }
+      end
+
+      context 'when there is a deployment record with canceled status' do
+        let!(:deployment) { create(:deployment, :canceled, environment: environment) }
+
+        it { is_expected.to eq(deployment) }
+      end
+    end
+  end
+
   describe '#has_terminals?' do
     subject { environment.has_terminals? }