Fix previous deployment

This commit fixes the previous deployment method, which was
wrongly implemented that it was taking the "other but latest"
deployment.

Now it's taking the previous deployment from the current pointer.

app/models/deployment.rb

@@ -239,17 +239,9 @@ class Deployment < ApplicationRecord
 
   def previous_deployment
     @previous_deployment ||=
-      self.class.for_environment(environment_id)
-      .where(ref: ref)
-      .where.not(id: id)
-      .order(id: :desc)
-      .take
-  end
-
-  def previous_environment_deployment
       self.class.for_environment(environment_id)
         .success
-      .where.not(id: self.id)
+        .where('id < ?', id)
         .order(id: :desc)
         .take
   end

app/services/deployments/link_merge_requests_service.rb

@@ -33,7 +33,7 @@ module Deployments
       # meaningful way (i.e. they can't just retry the deploy themselves).
       return unless deployment.success?
 
-      if (prev = deployment.previous_environment_deployment)
+      if (prev = deployment.previous_deployment)
         link_merge_requests_for_range(prev.sha, deployment.sha)
       else
         # When no previous deployment is found we fall back to linking all merge

changelogs/unreleased/fix-previous-deployment.yml

+---
+title: Fix previous deployment fetches wrong deployment
+merge_request: 58567
+author:
+type: fixed

spec/models/deployment_spec.rb

@@ -573,27 +573,39 @@ RSpec.describe Deployment do
   end
 
   describe '#previous_deployment' do
-    it 'returns the previous deployment' do
-      deploy1 = create(:deployment, :success)
-      deploy2 = create(
-        :deployment,
-        project: deploy1.project,
-        environment: deploy1.environment
-      )
+    using RSpec::Parameterized::TableSyntax
 
-      expect(deploy2.previous_deployment).to eq(deploy1)
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:production) { create(:environment, :production, project: project) }
+    let_it_be(:staging) { create(:environment, :staging, project: project) }
+    let_it_be(:production_deployment_1) { create(:deployment, :success, project: project, environment: production) }
+    let_it_be(:production_deployment_2) { create(:deployment, :success, project: project, environment: production) }
+    let_it_be(:production_deployment_3) { create(:deployment, :failed, project: project, environment: production) }
+    let_it_be(:production_deployment_4) { create(:deployment, :canceled, project: project, environment: production) }
+    let_it_be(:staging_deployment_1)    { create(:deployment, :failed, project: project, environment: staging) }
+    let_it_be(:staging_deployment_2)    { create(:deployment, :success, project: project, environment: staging) }
+    let_it_be(:production_deployment_5) { create(:deployment, :success, project: project, environment: production) }
+    let_it_be(:staging_deployment_3)    { create(:deployment, :success, project: project, environment: staging) }
+
+    where(:pointer, :expected_previous_deployment) do
+      'production_deployment_1'   | nil
+      'production_deployment_2'   | 'production_deployment_1'
+      'production_deployment_3'   | 'production_deployment_2'
+      'production_deployment_4'   | 'production_deployment_2'
+      'staging_deployment_1'      | nil
+      'staging_deployment_2'      | nil
+      'production_deployment_5'   | 'production_deployment_2'
+      'staging_deployment_3'      | 'staging_deployment_2'
+    end
+
+    with_them do
+      it 'returns the previous deployment' do
+        if expected_previous_deployment.nil?
+          expect(send(pointer).previous_deployment).to eq(expected_previous_deployment)
+        else
+          expect(send(pointer).previous_deployment).to eq(send(expected_previous_deployment))
+        end
       end
-
-    it 'returns nothing if the refs do not match' do
-      deploy1 = create(:deployment, :success)
-      deploy2 = create(
-        :deployment,
-        :review_app,
-        project: deploy1.project,
-        environment: deploy1.environment
-      )
-
-      expect(deploy2.previous_deployment).to be_nil
     end
   end
 
@@ -643,45 +655,6 @@ RSpec.describe Deployment do
     end
   end
 
-  describe '#previous_environment_deployment' do
-    it 'returns the previous deployment of the same environment' do
-      deploy1 = create(:deployment, :success)
-      deploy2 = create(
-        :deployment,
-        :success,
-        project: deploy1.project,
-        environment: deploy1.environment
-      )
-
-      expect(deploy2.previous_environment_deployment).to eq(deploy1)
-    end
-
-    it 'ignores deployments that were not successful' do
-      deploy1 = create(:deployment, :failed)
-      deploy2 = create(
-        :deployment,
-        :success,
-        project: deploy1.project,
-        environment: deploy1.environment
-      )
-
-      expect(deploy2.previous_environment_deployment).to be_nil
-    end
-
-    it 'ignores deployments for different environments' do
-      deploy1 = create(:deployment, :success)
-      preprod = create(:environment, project: deploy1.project, name: 'preprod')
-      deploy2 = create(
-        :deployment,
-        :success,
-        project: deploy1.project,
-        environment: preprod
-      )
-
-      expect(deploy2.previous_environment_deployment).to be_nil
-    end
-  end
-
   describe '#create_ref' do
     let(:deployment) { build(:deployment) }