Upgrade act as list gem, implement tests to check deal position behaviour and fix bug when change deal between stages from position 1

Gemfile

@@ -34,7 +34,7 @@ gem 'google-cloud-storage', require: false
 gem 'image_processing'
 
 # Authentication
-gem 'acts_as_list'
+gem 'acts_as_list', '1.2.6'
 gem 'acts-as-taggable-on', '12.0.0'
 gem 'cocoon'
 gem 'csv'

Gemfile.lock

@@ -96,7 +96,7 @@ GEM
     acts-as-taggable-on (12.0.0)
       activerecord (>= 7.1, < 8.1)
       zeitwerk (>= 2.4, < 3.0)
-    acts_as_list (1.2.2)
+    acts_as_list (1.2.6)
       activerecord (>= 6.1)
       activesupport (>= 6.1)
     addressable (2.8.7)
@@ -807,7 +807,7 @@ PLATFORMS
 
 DEPENDENCIES
   acts-as-taggable-on (= 12.0.0)
-  acts_as_list
+  acts_as_list (= 1.2.6)
   annotate
   aws-sdk-s3
   azure-storage-blob!

app/models/deal.rb

@@ -7,7 +7,7 @@
 #  lost_at                             :datetime
 #  lost_reason                         :string           default(""), not null
 #  name                                :string           default(""), not null
-#  position                            :integer          default(1), not null
+#  position                            :integer
 #  status                              :string           default("open"), not null
 #  total_deal_products_amount_in_cents :bigint           default(0), not null
 #  won_at                              :datetime

app/models/stage.rb

@@ -4,7 +4,7 @@
 #
 #  id          :bigint           not null, primary key
 #  name        :string           default(""), not null
-#  position    :integer          default(1), not null
+#  position    :integer
 #  created_at  :datetime         not null
 #  updated_at  :datetime         not null
 #  pipeline_id :bigint           not null

db/migrate/20260213000000_remove_default_and_null_from_position_on_deals_and_stages.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class RemoveDefaultAndNullFromPositionOnDealsAndStages < ActiveRecord::Migration[7.1]
+  def change
+    change_column_default :deals, :position, from: 1, to: nil
+    change_column_null :deals, :position, true
+
+    change_column_default :stages, :position, from: 1, to: nil
+    change_column_null :stages, :position, true
+  end
+end

spec/controllers/accounts/deals_controller_spec.rb

@@ -5,7 +5,6 @@
   let!(:user) { create(:user) }
   let!(:pipeline) { create(:pipeline) }
   let!(:stage) { create(:stage, pipeline:) }
-  let!(:stage_2) { create(:stage, pipeline:, name: 'Stage 2') }
   let!(:contact) { create(:contact) }
   let(:event) { create(:event, deal:, kind: 'activity') }
   let(:last_event) { Event.last }
@@ -73,69 +72,6 @@
           expect(response).to have_http_status(:redirect)
         end
       end
-      context 'update deal position and create deal_stage_change event' do
-        around(:each) do |example|
-          Sidekiq::Testing.inline! do
-            example.run
-          end
-        end
-        let!(:deal_stage_1_position_1) { create(:deal, stage:, position: 1) }
-        let!(:deal_stage_1_position_2) { create(:deal, stage:, position: 2) }
-        let!(:deal_stage_1_position_3) { create(:deal, stage:, position: 3) }
-        let!(:deal_stage_2_position_1) { create(:deal, stage: stage_2, position: 1) }
-        let!(:deal_stage_2_position_2) { create(:deal, stage: stage_2, position: 2) }
-        let!(:deal_stage_2_position_3) { create(:deal, stage: stage_2, position: 3) }
-        skip 'between different stages' do
-          it 'stage 1 position 3 to stage 2 position 1' do
-            params =  { deal: { stage_id: stage_2.id, position: 1 } }
-
-            put("/accounts/#{account.id}/deals/#{deal_stage_1_position_3.id}",
-                params:)
-            # expect(response).to have_http_status(:success)
-            expect(deal_stage_1_position_3.reload.position).to eq(1)
-            expect(deal_stage_1_position_3.reload.stage).to eq(stage_2)
-            expect(deal_stage_2_position_1.reload.position).to eq(2)
-          end
-          it 'stage 1 position 1 to stage 2 position 1' do
-            params =  { deal: { stage_id: stage_2.id, position: 1 } }
-            expect do
-              put("/accounts/#{account.id}/deals/#{deal_stage_1_position_1.id}",
-                  params:)
-            end.to change(Event, :count).by(1)
-            # expect(response).to have_http_status(:success)
-            expect(deal_stage_1_position_1.reload.position).to eq(1)
-            expect(deal_stage_1_position_1.reload.stage).to eq(stage_2)
-            expect(deal_stage_2_position_1.reload.position).to eq(2)
-            expect(last_event.kind).to eq('deal_stage_change')
-          end
-        end
-        context 'in the same stage' do
-          it 'position 3 to position 1' do
-            params = { deal: { stage_id: stage.id, position: 1 } }
-            put("/accounts/#{account.id}/deals/#{deal_stage_1_position_3.id}",
-                params:)
-            # expect(response).to have_http_status(:success)
-            expect(deal_stage_1_position_3.reload.position).to eq(1)
-            expect(deal_stage_1_position_3.reload.stage).to eq(stage)
-          end
-          it 'position 1 to position 3' do
-            params = { deal: { stage_id: stage.id, position: 3 } }
-            put("/accounts/#{account.id}/deals/#{deal_stage_1_position_1.id}",
-                params:)
-            # expect(response).to have_http_status(:success)
-            expect(deal_stage_1_position_1.reload.position).to eq(3)
-            expect(deal_stage_1_position_1.reload.stage).to eq(stage)
-          end
-          it 'position 2 to position 1' do
-            params = { deal: { stage_id: stage.id, position: 1 } }
-            put("/accounts/#{account.id}/deals/#{deal_stage_1_position_2.id}",
-                params:)
-            # expect(response).to have_http_status(:success)
-            expect(deal_stage_1_position_2.reload.position).to eq(1)
-            expect(deal_stage_1_position_2.reload.stage).to eq(stage)
-          end
-        end
-      end
 
       context 'update status deal' do
         it 'update to won and create deal_won event' do

spec/factories/deals.rb

@@ -7,7 +7,7 @@
 #  lost_at                             :datetime
 #  lost_reason                         :string           default(""), not null
 #  name                                :string           default(""), not null
-#  position                            :integer          default(1), not null
+#  position                            :integer
 #  status                              :string           default("open"), not null
 #  total_deal_products_amount_in_cents :bigint           default(0), not null
 #  won_at                              :datetime

spec/factories/stages.rb

@@ -4,7 +4,7 @@
 #
 #  id          :bigint           not null, primary key
 #  name        :string           default(""), not null
-#  position    :integer          default(1), not null
+#  position    :integer
 #  created_at  :datetime         not null
 #  updated_at  :datetime         not null
 #  pipeline_id :bigint           not null

spec/models/deals_spec.rb

@@ -0,0 +1,134 @@
+require 'rails_helper'
+
+RSpec.describe Deal do
+  let!(:account) { create(:account) }
+  let(:last_event) { Event.last }
+
+  describe 'position' do
+    let!(:pipeline) { create(:pipeline) }
+    let!(:stage1) { create(:stage, pipeline:) }
+    let!(:stage2) { create(:stage, pipeline:) }
+
+    let!(:stage1_position1) { create(:deal, stage: stage1, position: 1) }
+    let!(:stage1_position2) { create(:deal, stage: stage1, position: 2) }
+    let!(:stage1_position3) { create(:deal, stage: stage1, position: 3) }
+
+    let!(:stage2_position1) { create(:deal, stage: stage2, position: 1) }
+    let!(:stage2_position2) { create(:deal, stage: stage2, position: 2) }
+    let!(:stage2_position3) { create(:deal, stage: stage2, position: 3) }
+
+    context 'moving between stages' do
+      it 'move stage1_position1 to stage2 position 1 and create deal_stage_change event' do
+        expect do
+          Deal::CreateOrUpdate.new(stage1_position1, { stage: stage2, position: 1 }).call
+        end.to change(Event, :count).by(1)
+
+        expect(stage1_position2.reload.position).to eq 1
+        expect(stage1_position3.reload.position).to eq 2
+
+        expect(stage1_position1.reload.stage_id).to eq stage2.id
+
+        expect(stage1_position1.position).to eq 1
+        expect(stage2_position1.reload.position).to eq 2
+        expect(stage2_position2.reload.position).to eq 3
+        expect(stage2_position3.reload.position).to eq 4
+        expect(last_event.kind).to eq('deal_stage_change')
+      end
+
+      it 'move stage1_position1 to stage2 position 2 and create deal_stage_change event' do
+        expect do
+          Deal::CreateOrUpdate.new(stage1_position1, { stage: stage2, position: 2 }).call
+        end.to change(Event, :count).by(1)
+
+        expect(stage1_position2.reload.position).to eq 1
+        expect(stage1_position3.reload.position).to eq 2
+
+        expect(stage1_position1.reload.stage_id).to eq stage2.id
+
+        expect(stage2_position1.reload.position).to eq 1
+        expect(stage1_position1.position).to eq 2
+        expect(stage2_position2.reload.position).to eq 3
+        expect(stage2_position3.reload.position).to eq 4
+        expect(last_event.kind).to eq('deal_stage_change')
+      end
+
+      it 'move stage1_position1 to stage2 position 3 and create deal_stage_change event' do
+        expect do
+          Deal::CreateOrUpdate.new(stage1_position1, { stage: stage2, position: 3 }).call
+        end.to change(Event, :count).by(1)
+
+        expect(stage1_position2.reload.position).to eq 1
+        expect(stage1_position3.reload.position).to eq 2
+
+        expect(stage1_position1.reload.stage_id).to eq stage2.id
+
+        expect(stage2_position1.reload.position).to eq 1
+        expect(stage2_position2.reload.position).to eq 2
+        expect(stage1_position1.position).to eq 3
+        expect(stage2_position3.reload.position).to eq 4
+        expect(last_event.kind).to eq('deal_stage_change')
+      end
+
+      it 'move stage1_position1 to stage2 position 250 and create deal_stage_change event' do
+        expect do
+          Deal::CreateOrUpdate.new(stage1_position1, { stage: stage2, position: 250 }).call
+        end.to change(Event, :count).by(1)
+
+        expect(stage1_position2.reload.position).to eq 1
+        expect(stage1_position3.reload.position).to eq 2
+
+        expect(stage1_position1.reload.stage_id).to eq stage2.id
+
+        expect(stage2_position1.reload.position).to eq 1
+        expect(stage2_position2.reload.position).to eq 2
+        expect(stage2_position3.reload.position).to eq 3
+        expect(stage1_position1.position).to eq 250
+        expect(last_event.kind).to eq('deal_stage_change')
+      end
+    end
+
+    context 'moving within the same stage' do
+      it 'move stage1_position1 to position 2' do
+        Deal::CreateOrUpdate.new(stage1_position1, { position: 2 }).call
+
+        expect(stage1_position1.reload.position).to eq 2
+        expect(stage1_position2.reload.position).to eq 1
+        expect(stage1_position3.reload.position).to eq 3
+      end
+
+      it 'move stage1_position1 to position 3' do
+        Deal::CreateOrUpdate.new(stage1_position1, { position: 3 }).call
+
+        expect(stage1_position1.reload.position).to eq 3
+        expect(stage1_position2.reload.position).to eq 1
+        expect(stage1_position3.reload.position).to eq 2
+      end
+    end
+
+    context 'create a deal' do
+      context 'when position is not provided' do
+        it 'places the new deal at the end of the stage' do
+          new_deal = build(:deal, stage: stage1)
+          new_deal = Deal::CreateOrUpdate.new(new_deal, {}).call
+
+          expect(stage1_position1.reload.position).to eq 1
+          expect(stage1_position2.reload.position).to eq 2
+          expect(stage1_position3.reload.position).to eq 3
+          expect(new_deal.reload.position).to eq 4
+        end
+      end
+
+      context 'when position is provided' do
+        it 'inserts the new deal at the specified position' do
+          new_deal = build(:deal, stage: stage1, position: 2)
+          new_deal = Deal::CreateOrUpdate.new(new_deal, {}).call
+
+          expect(stage1_position1.reload.position).to eq 1
+          expect(new_deal.reload.position).to eq 2
+          expect(stage1_position2.reload.position).to eq 3
+          expect(stage1_position3.reload.position).to eq 4
+        end
+      end
+    end
+  end
+end

spec/models/stage_spec.rb

@@ -1,3 +1,22 @@
+# == Schema Information
+#
+# Table name: stages
+#
+#  id          :bigint           not null, primary key
+#  name        :string           default(""), not null
+#  position    :integer
+#  created_at  :datetime         not null
+#  updated_at  :datetime         not null
+#  pipeline_id :bigint           not null
+#
+# Indexes
+#
+#  index_stages_on_pipeline_id  (pipeline_id)
+#
+# Foreign Keys
+#
+#  fk_rails_...  (pipeline_id => pipelines.id)
+#
 require 'rails_helper'
 
 RSpec.describe Stage do