Merge branch 'ali/expose-deployment-approvals-to-internal-environments-api' into 'master'

Expose Deployment Approvals to internal Environments API

See merge request gitlab-org/gitlab!81487

app/models/preloaders/environments/deployment_preloader.rb

@@ -34,8 +34,16 @@ module Preloaders
         deployments_by_environment_id = deployments.index_by(&:environment_id)
 
         environments.each do |environment|
-          environment.association(association_name).target = deployments_by_environment_id[environment.id]
+          associated_deployment = deployments_by_environment_id[environment.id]
+
+          environment.association(association_name).target = associated_deployment
           environment.association(association_name).loaded!
+
+          if associated_deployment
+            # `last?` in DeploymentEntity requires this environment to be loaded
+            associated_deployment.association(:environment).target = environment
+            associated_deployment.association(:environment).loaded!
+          end
         end
       end
     end

doc/api/deployments.md

@@ -281,7 +281,8 @@ Deployments created by users on GitLab Premium or higher include the `approvals`
         "avatar_url": "https://www.gravatar.com/avatar/e83ac685f68ea07553ad3054c738c709?s=80&d=identicon",
         "web_url": "http://localhost:3000/project_6_bot"
       },
-      "status": "approved"
+      "status": "approved",
+      "created_at": "2022-02-24T20:22:30.097Z"
     }
   ],
   ...
@@ -351,7 +352,8 @@ Deployments created by users on GitLab Premium or higher include the `approvals`
         "avatar_url": "https://www.gravatar.com/avatar/e83ac685f68ea07553ad3054c738c709?s=80&d=identicon",
         "web_url": "http://localhost:3000/project_6_bot"
       },
-      "status": "approved"
+      "status": "approved",
+      "created_at": "2022-02-24T20:22:30.097Z"
     }
   ],
   ...
@@ -417,7 +419,8 @@ Deployments created by users on GitLab Premium or higher include the `approvals`
         "avatar_url": "https://www.gravatar.com/avatar/e83ac685f68ea07553ad3054c738c709?s=80&d=identicon",
         "web_url": "http://localhost:3000/project_6_bot"
       },
-      "status": "approved"
+      "status": "approved",
+      "created_at": "2022-02-24T20:22:30.097Z"
     }
   ],
   ...
@@ -480,6 +483,7 @@ Example response:
     "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
     "web_url": "http://localhost:3000/root"
   },
-  "status": "approved"
+  "status": "approved",
+  "created_at": "2022-02-24T20:22:30.097Z"
 }
 ```

ee/app/models/ee/deployment.rb

@@ -34,7 +34,7 @@ module EE
     def pending_approval_count
       return 0 unless blocked?
 
-      environment.required_approval_count - approvals.count
+      environment.required_approval_count - approvals.length
     end
   end
 end

ee/app/serializers/ee/deployment_entity.rb

@@ -6,6 +6,7 @@ module EE
 
     prepended do
       expose :pending_approval_count
+      expose :approvals, using: ::API::Entities::Deployments::Approval
     end
   end
 end

ee/app/serializers/ee/environment_serializer.rb

@@ -10,6 +10,13 @@ module EE
       super.deep_merge(latest_opened_most_severe_alert: [])
     end
 
+    override :deployment_associations
+    def deployment_associations
+      super.deep_merge(approvals: {
+        user: []
+      })
+    end
+
     override :project_associations
     def project_associations
       super.deep_merge(protected_environments: [])

ee/lib/api/entities/deployments/approval.rb

@@ -6,6 +6,7 @@ module API
       class Approval < Grape::Entity
         expose :user, using: Entities::UserBasic
         expose :status
+        expose :created_at
       end
     end
   end

ee/spec/fixtures/api/schemas/public_api/v4/deployment_approval.json

@@ -13,6 +13,10 @@
       "items": {
         "$ref": "../../../../../../../spec/fixtures/api/schemas/public_api/v4/user/basic.json"
       }
+    },
+    "created_at": {
+      "type": "string",
+      "format": "date-time"
     }
   },
   "additionalProperties": false

ee/spec/lib/api/entities/deployments/approval_spec.rb

@@ -8,6 +8,6 @@ RSpec.describe API::Entities::Deployments::Approval do
   let(:approval) { build(:deployment_approval) }
 
   it 'exposes correct attributes' do
-    expect(subject.keys).to contain_exactly(:user, :status)
+    expect(subject.keys).to contain_exactly(:user, :status, :created_at)
   end
 end

ee/spec/models/deployment_spec.rb

@@ -69,6 +69,17 @@ RSpec.describe Deployment do
           expect(deployment.pending_approval_count).to eq(0)
         end
       end
+
+      context 'loading approval count' do
+        before do
+          deployment.environment.required_approval_count
+          deployment.approvals.to_a
+        end
+
+        it 'does not perform an extra query when approvals are loaded', :request_store do
+          expect { deployment.pending_approval_count }.not_to exceed_query_limit(0)
+        end
+      end
     end
 
     context 'when Protected Environments feature is not available' do

ee/spec/serializers/ee/deployment_entity_spec.rb

@@ -21,4 +21,10 @@ RSpec.describe DeploymentEntity do
       expect(subject[:pending_approval_count]).to eq(2)
     end
   end
+
+  describe '#approvals' do
+    it 'exposes approvals' do
+      expect(subject[:approvals].length).to eq(1)
+    end
+  end
 end

ee/spec/serializers/ee/environment_serializer_spec.rb

@@ -19,8 +19,10 @@ RSpec.describe EE::EnvironmentSerializer do
   def create_environment_with_associations(project)
     create(:environment, project: project).tap do |environment|
       create(:deployment, :success, environment: environment, project: project)
-      create(:deployment, :running, environment: environment, project: project)
-      create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project)
+      create(:deployment, :blocked, environment: environment, project: project) do |deployment|
+        create(:deployment_approval, deployment: deployment)
+      end
+      create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project, required_approval_count: 2)
       prometheus_alert = create(:prometheus_alert, project: project, environment: environment)
       create(:alert_management_alert, :triggered, :prometheus, project: project, environment: environment, prometheus_alert: prometheus_alert)
     end

spec/models/preloaders/environments/deployment_preloader_spec.rb

@@ -62,4 +62,22 @@ RSpec.describe Preloaders::Environments::DeploymentPreloader do
 
     expect(default_preload_query).to be(false)
   end
+
+  it 'sets environment on the associated deployment', :aggregate_failures do
+    preload_association(:last_deployment)
+
+    expect do
+      project.environments.each { |environment| environment.last_deployment.environment }
+    end.not_to exceed_query_limit(0)
+
+    project.environments.each do |environment|
+      expect(environment.last_deployment.environment).to eq(environment)
+    end
+  end
+
+  it 'does not attempt to set environment on a nil deployment' do
+    create(:environment, project: project, state: :available)
+
+    expect { preload_association(:last_deployment) }.not_to raise_error
+  end
 end

spec/support/shared_examples/serializers/environment_serializer_shared_examples.rb

 # frozen_string_literal: true
 RSpec.shared_examples 'avoid N+1 on environments serialization' do |ee: false|
   # Investigating in https://gitlab.com/gitlab-org/gitlab/-/issues/353209
-  let(:query_threshold) { 9 + (ee ? 4 : 0) }
+  let(:query_threshold) { 1 + (ee ? 4 : 0) }
 
   it 'avoids N+1 database queries with grouping', :request_store do
     create_environment_with_associations(project)