From 432b1c5c1e99eb776fc8b0b006113a71015ec1b7 Mon Sep 17 00:00:00 2001 From: Yukio Arie Date: Fri, 13 Feb 2026 02:25:19 -0300 Subject: [PATCH 1/7] Update acts_as_list gem --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 1244a773f..286c7a745 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 7ece059ce..e2f40157c 100644 --- a/Gemfile.lock +++ b/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! From 25e5027bc9914517c5f1473b900263f6d47614d5 Mon Sep 17 00:00:00 2001 From: Yukio Arie Date: Fri, 13 Feb 2026 02:25:33 -0300 Subject: [PATCH 2/7] Implement tests to check deal position behaviour --- spec/models/deals_spec.rb | 85 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 spec/models/deals_spec.rb diff --git a/spec/models/deals_spec.rb b/spec/models/deals_spec.rb new file mode 100644 index 000000000..9224f86b5 --- /dev/null +++ b/spec/models/deals_spec.rb @@ -0,0 +1,85 @@ +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.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(stage2_position1.reload.position).to eq 1 + expect(stage1_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 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(stage2_position1.reload.position).to eq 1 + expect(stage2_position2.reload.position).to eq 2 + expect(stage1_position1.reload.position).to eq 3 + expect(stage2_position3.reload.position).to eq 4 + 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 + end +end From 03c86991987231a5af5da9856946704461e8f42a Mon Sep 17 00:00:00 2001 From: Yukio Arie Date: Fri, 13 Feb 2026 02:26:01 -0300 Subject: [PATCH 3/7] Remove deals positions behaviour tests in deals controller --- .../accounts/deals_controller_spec.rb | 64 ------------------- 1 file changed, 64 deletions(-) diff --git a/spec/controllers/accounts/deals_controller_spec.rb b/spec/controllers/accounts/deals_controller_spec.rb index fa02fcbba..ea2ba7487 100644 --- a/spec/controllers/accounts/deals_controller_spec.rb +++ b/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 From fdfaf831607e932e62b59425d4426f0c2d310d2e Mon Sep 17 00:00:00 2001 From: Yukio Arie Date: Fri, 13 Feb 2026 06:09:26 -0300 Subject: [PATCH 4/7] Gerenate migration to fix bug when moving an deal between stages when deal position is 1 --- app/models/deal.rb | 2 +- app/models/stage.rb | 2 +- ..._null_from_position_on_deals_and_stages.rb | 11 +++++++++++ db/schema.rb | 6 +++--- spec/factories/deals.rb | 2 +- spec/factories/stages.rb | 2 +- spec/models/stage_spec.rb | 19 +++++++++++++++++++ 7 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20260213000000_remove_default_and_null_from_position_on_deals_and_stages.rb diff --git a/app/models/deal.rb b/app/models/deal.rb index bf7f086e2..6c77102f4 100644 --- a/app/models/deal.rb +++ b/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 diff --git a/app/models/stage.rb b/app/models/stage.rb index f05b54048..fafaa07ab 100644 --- a/app/models/stage.rb +++ b/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 diff --git a/db/migrate/20260213000000_remove_default_and_null_from_position_on_deals_and_stages.rb b/db/migrate/20260213000000_remove_default_and_null_from_position_on_deals_and_stages.rb new file mode 100644 index 000000000..1365372d8 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index f6aa91b25..3919c9050 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2025_10_17_013421) do +ActiveRecord::Schema[7.1].define(version: 2026_02_13_000000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -187,7 +187,7 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.bigint "pipeline_id" - t.integer "position", default: 1, null: false + t.integer "position" t.integer "created_by_id" t.bigint "total_deal_products_amount_in_cents", default: 0, null: false t.datetime "lost_at" @@ -545,7 +545,7 @@ create_table "stages", force: :cascade do |t| t.string "name", default: "", null: false t.bigint "pipeline_id", null: false - t.integer "position", default: 1, null: false + t.integer "position" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["pipeline_id"], name: "index_stages_on_pipeline_id" diff --git a/spec/factories/deals.rb b/spec/factories/deals.rb index bea0bf53c..06375d333 100644 --- a/spec/factories/deals.rb +++ b/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 diff --git a/spec/factories/stages.rb b/spec/factories/stages.rb index a495d8a89..94df246ee 100644 --- a/spec/factories/stages.rb +++ b/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 diff --git a/spec/models/stage_spec.rb b/spec/models/stage_spec.rb index 9466116bf..19a03ece0 100644 --- a/spec/models/stage_spec.rb +++ b/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 From f7dca40b88c48083707560a6c06513140bf14daa Mon Sep 17 00:00:00 2001 From: Yukio Arie Date: Sun, 15 Feb 2026 20:06:25 -0300 Subject: [PATCH 5/7] Improve and add tests --- spec/models/deals_spec.rb | 55 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/spec/models/deals_spec.rb b/spec/models/deals_spec.rb index 9224f86b5..886ebcb50 100644 --- a/spec/models/deals_spec.rb +++ b/spec/models/deals_spec.rb @@ -26,7 +26,9 @@ expect(stage1_position2.reload.position).to eq 1 expect(stage1_position3.reload.position).to eq 2 - expect(stage1_position1.reload.position).to eq 1 + 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 @@ -41,8 +43,10 @@ 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.reload.position).to eq 2 + 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') @@ -56,12 +60,31 @@ 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.reload.position).to eq 3 + 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 @@ -81,5 +104,31 @@ expect(stage1_position3.reload.position).to eq 2 end end + + context 'create a deal' do + context 'when position is 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 not 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 From f4a2ac1b2bc780f2d4395ecccc273cedb22fff06 Mon Sep 17 00:00:00 2001 From: Douglas Lara Date: Tue, 17 Feb 2026 08:18:38 +1000 Subject: [PATCH 6/7] Update spec/models/deals_spec.rb --- spec/models/deals_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/deals_spec.rb b/spec/models/deals_spec.rb index 886ebcb50..c974b9eff 100644 --- a/spec/models/deals_spec.rb +++ b/spec/models/deals_spec.rb @@ -106,7 +106,7 @@ end context 'create a deal' do - context 'when position is provided' 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 From f3f53e80ccb671ac2965a21143e8eab0f1494eef Mon Sep 17 00:00:00 2001 From: Douglas Lara Date: Tue, 17 Feb 2026 08:18:45 +1000 Subject: [PATCH 7/7] Update spec/models/deals_spec.rb --- spec/models/deals_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/deals_spec.rb b/spec/models/deals_spec.rb index c974b9eff..34ea6d46a 100644 --- a/spec/models/deals_spec.rb +++ b/spec/models/deals_spec.rb @@ -118,7 +118,7 @@ end end - context 'when position is not provided' do + 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