Merge branch 'ali/deployment-approvals-mvc-2' into 'master'

Deployment Approval service class

See merge request gitlab-org/gitlab!75710

ee/app/models/ee/deployment.rb

@@ -31,13 +31,6 @@ module EE
       end
     end
 
-    override :sync_status_with
-    def sync_status_with(build)
-      return update_status!('blocked') if build.status == 'manual' && needs_approval?
-
-      super
-    end
-
     def pending_approval_count
       environment.required_approval_count - approvals.approved.count
     end

ee/app/services/deployments/approval_service.rb

+# frozen_string_literal: true
+
+module Deployments
+  class ApprovalService < ::BaseService
+    def execute(deployment, status)
+      error_message = validate(deployment, status)
+      return error(error_message) if error_message
+
+      approval = upsert_approval(deployment, status)
+      return error(approval.errors.full_messages) if approval.errors.any?
+
+      process_build!(deployment, approval)
+
+      success(approval: approval)
+    end
+
+    private
+
+    def upsert_approval(deployment, status)
+      if (approval = deployment.approvals.find_by_user_id(current_user.id))
+        return approval if approval.status == status
+
+        approval.tap { |a| a.update(status: status) }
+      else
+        deployment.approvals.create(user: current_user, status: status)
+      end
+    end
+
+    def process_build!(deployment, approval)
+      return unless deployment.deployable
+
+      if approval.rejected?
+        deployment.deployable.drop!(:deployment_rejected)
+      elsif deployment.pending_approval_count <= 0
+        deployment.deployable.enqueue!
+      end
+    end
+
+    def validate(deployment, status)
+      return 'Unrecognized status' unless Deployments::Approval.statuses.include?(status)
+
+      return 'This environment is not protected' unless deployment.environment.protected?
+
+      return 'You do not have permission to approve or reject this deployment' unless current_user&.can?(:update_deployment, deployment)
+
+      return 'This deployment job is not waiting for approvals' unless deployment.blocked?
+
+      'The same user can not approve' if deployment.user == current_user && status == 'approved'
+    end
+  end
+end

ee/app/services/ee/ci/process_build_service.rb

@@ -6,8 +6,12 @@ module EE
 
       override :enqueue
       def enqueue(build)
+        # TODO: Refactor to check these conditions before actionizing or scheduling a build. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75710#note_756445822
         if build.persisted_environment.try(:protected_from?, build.user)
           return build.drop!(:protected_environment_failure)
+        elsif build.persisted_environment.try(:needs_approval?)
+          build.actionize!
+          return build.deployment&.block!
         end
 
         super

ee/spec/factories/deployments/approval.rb

@@ -4,10 +4,10 @@ FactoryBot.define do
   factory :deployment_approval, class: 'Deployments::Approval' do
     user
     deployment
-    status { :approved }
+    status { 'approved' }
 
     trait :rejected do
-      status { :rejected }
+      status { 'rejected' }
     end
   end
 end

ee/spec/models/deployment_spec.rb

@@ -24,67 +24,6 @@ RSpec.describe Deployment do
     end
   end
 
-  describe '#sync_status_with' do
-    subject { deployment.sync_status_with(ci_build) }
-
-    let_it_be(:project) { create(:project, :repository) }
-
-    let(:environment) { create(:environment, project: project) }
-    let(:deployment) { create(:deployment, project: project, environment: environment) }
-
-    context 'when build is manual' do
-      let(:ci_build) { create(:ci_build, project: project, status: :manual) }
-
-      context 'and Protected Environments feature is available' do
-        before do
-          stub_licensed_features(protected_environments: true)
-          create(:protected_environment, name: environment.name, project: project, required_approval_count: required_approval_count)
-        end
-
-        context 'and deployment needs approval' do
-          let(:required_approval_count) { 1 }
-
-          it 'blocks the deployment' do
-            expect(Gitlab::ErrorTracking).not_to receive(:track_exception)
-
-            is_expected.to eq(true)
-
-            expect(deployment.status).to eq('blocked')
-            expect(deployment.errors).to be_empty
-          end
-        end
-
-        context 'and deployment does not need approval' do
-          let(:required_approval_count) { 0 }
-
-          it 'does not change the deployment' do
-            expect(Gitlab::ErrorTracking).not_to receive(:track_exception)
-
-            is_expected.to eq(false)
-
-            expect(deployment.status).to eq('created')
-            expect(deployment.errors).to be_empty
-          end
-        end
-      end
-
-      context 'and Protected Environments feature is not available' do
-        before do
-          stub_licensed_features(protected_environments: false)
-        end
-
-        it 'does not change the deployment' do
-          expect(Gitlab::ErrorTracking).not_to receive(:track_exception)
-
-          is_expected.to eq(false)
-
-          expect(deployment.status).to eq('created')
-          expect(deployment.errors).to be_empty
-        end
-      end
-    end
-  end
-
   describe '#pending_approval_count' do
     let_it_be(:project) { create(:project, :repository) }
 

ee/spec/services/ci/process_build_service_spec.rb

@@ -52,6 +52,28 @@ RSpec.describe Ci::ProcessBuildService, '#execute' do
 
           expect(ci_build.pending?).to be_truthy
         end
+
+        context 'and environment needs approval' do
+          before do
+            protected_environment.update!(required_approval_count: 1)
+          end
+
+          it 'makes the build a manual action' do
+            expect { subject }.to change { ci_build.status }.from('created').to('manual')
+          end
+
+          context 'and the build has a deployment' do
+            let(:deployment) { create(:deployment, deployable: ci_build, environment: environment, user: user, project: project) }
+
+            it 'blocks the deployment' do
+              expect { subject }.to change { deployment.reload.status }.from('created').to('blocked')
+            end
+
+            it 'makes the build a manual action' do
+              expect { subject }.to change { ci_build.status }.from('created').to('manual')
+            end
+          end
+        end
       end
     end
   end

ee/spec/services/deployments/approval_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Deployments::ApprovalService do
+  let_it_be(:project) { create(:project, :repository) }
+
+  let(:service) { described_class.new(project, user) }
+  let(:user) { create(:user) }
+  let(:environment) { create(:environment, project: project) }
+  let(:status) { 'approved' }
+  let(:required_approval_count) { 2 }
+  let(:build) { create(:ci_build, :manual, project: project) }
+  let(:deployment) { create(:deployment, :blocked, project: project, environment: environment, deployable: build) }
+
+  before do
+    stub_licensed_features(protected_environments: true)
+    create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project, required_approval_count: required_approval_count)
+    project.add_maintainer(user)
+  end
+
+  shared_examples_for 'error' do |message:|
+    it 'returns an error' do
+      expect(subject[:status]).to eq(:error)
+      expect(subject[:message]).to eq(message)
+    end
+  end
+
+  shared_examples_for 'reject' do
+    it 'rejects the deployment' do
+      expect(subject[:status]).to eq(:success)
+      expect(subject[:approval].status).to eq('rejected')
+      expect(subject[:approval].user).to eq(user)
+      expect(subject[:approval].deployment).to eq(deployment)
+      expect(deployment.approvals.approved.count).to eq(0)
+      expect(deployment.approvals.rejected.count).to eq(1)
+    end
+  end
+
+  shared_examples_for 'approve' do
+    it 'approves the deployment' do
+      expect(subject[:status]).to eq(:success)
+      expect(subject[:approval].status).to eq('approved')
+      expect(subject[:approval].user).to eq(user)
+      expect(subject[:approval].deployment).to eq(deployment)
+      expect(deployment.approvals.approved.count).to eq(1)
+      expect(deployment.approvals.rejected.count).to eq(0)
+    end
+  end
+
+  describe '#execute' do
+    subject { service.execute(deployment, status) }
+
+    context 'when status is approved' do
+      include_examples 'approve'
+    end
+
+    context 'when status is rejected' do
+      let(:status) { 'rejected' }
+
+      include_examples 'reject'
+    end
+
+    context 'when user already approved' do
+      before do
+        service.execute(deployment, :approved)
+      end
+
+      context 'and is approving again' do
+        include_examples 'approve'
+      end
+
+      context 'and is rejecting' do
+        let(:status) { 'rejected' }
+
+        include_examples 'reject'
+      end
+    end
+
+    context 'processing the build' do
+      context 'when build is nil' do
+        before do
+          deployment.deployable = nil
+        end
+
+        it 'does not raise an error' do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      context 'when deployment was rejected' do
+        let(:status) { 'rejected' }
+
+        it 'drops the build' do
+          subject
+
+          expect(deployment.deployable.status).to eq('failed')
+          expect(deployment.deployable.failure_reason).to eq('deployment_rejected')
+        end
+      end
+
+      context 'when no additional approvals are required' do
+        let(:required_approval_count) { 1 }
+
+        it 'enqueues the build' do
+          subject
+
+          expect(deployment.deployable.status).to eq('pending')
+        end
+      end
+
+      context 'when additional approvals are required' do
+        let(:required_approval_count) { 2 }
+
+        it 'does not change the build' do
+          expect { subject }.not_to change { deployment.deployable.reload.status }
+        end
+      end
+    end
+
+    context 'validations' do
+      context 'when status is not recognized' do
+        let(:status) { 'foo' }
+
+        include_examples 'error', message: 'Unrecognized status'
+      end
+
+      context 'when environment is not protected' do
+        let(:deployment) { create(:deployment, project: project, deployable: build) }
+
+        include_examples 'error', message: 'This environment is not protected'
+      end
+
+      context 'when Protected Environments feature is not available' do
+        before do
+          stub_licensed_features(protected_environments: false)
+        end
+
+        include_examples 'error', message: 'This environment is not protected'
+      end
+
+      context 'when the user does not have permission to update deployment' do
+        before do
+          project.add_developer(user)
+        end
+
+        include_examples 'error', message: 'You do not have permission to approve or reject this deployment'
+      end
+
+      context 'when user is nil' do
+        let(:user) { nil }
+
+        include_examples 'error', message: 'You do not have permission to approve or reject this deployment'
+      end
+
+      context 'when deployment is not blocked' do
+        let(:deployment) { create(:deployment, project: project, environment: environment, deployable: build) }
+
+        include_examples 'error', message: 'This deployment job is not waiting for approvals'
+      end
+
+      context 'when the creator of the deployment is approving' do
+        before do
+          deployment.user = user
+        end
+
+        include_examples 'error', message: 'The same user can not approve'
+      end
+
+      context 'when the creator of the deployment is rejecting' do
+        let(:status) { 'rejected' }
+
+        before do
+          deployment.user = user
+        end
+
+        include_examples 'reject'
+      end
+    end
+  end
+end

spec/factories/deployments.rb

@@ -55,6 +55,10 @@ FactoryBot.define do
       status { :created }
     end
 
+    trait :blocked do
+      status { :blocked }
+    end
+
     # This trait hooks the state maechine's events
     trait :succeed do
       after(:create) do |deployment, evaluator|