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! 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/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 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/deals_spec.rb b/spec/models/deals_spec.rb new file mode 100644 index 000000000..34ea6d46a --- /dev/null +++ b/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 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