From 516e17893d153458f2a39a58eb21cbbf9e0b11e8 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Tue, 30 Mar 2021 15:08:29 +0500 Subject: [PATCH 01/16] Create a migration to insert trail plans --- .../20210329102724_add_new_trail_plans.rb | 17 +++++++++++++++++ db/schema_migrations/20210329102724 | 1 + 2 files changed, 18 insertions(+) create mode 100644 db/post_migrate/20210329102724_add_new_trail_plans.rb create mode 100644 db/schema_migrations/20210329102724 diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb new file mode 100644 index 00000000000000..59649a2cdf3ee6 --- /dev/null +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddNewTrailPlans < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + execute "INSERT INTO plans (name, title, created_at, updated_at) VALUES ('premium-trial', 'Premium Trial', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" + execute "INSERT INTO plans (name, title, created_at, updated_at) VALUES ('ultimate-trial', 'Ultimate Trial', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" + end + + def down + execute "DELETE FROM plans WHERE name IN ('premium-trial', 'ultimate-trial')" + end +end diff --git a/db/schema_migrations/20210329102724 b/db/schema_migrations/20210329102724 new file mode 100644 index 00000000000000..b2fdccf2bb878d --- /dev/null +++ b/db/schema_migrations/20210329102724 @@ -0,0 +1 @@ +b40c702ea6b2120da6fe11b213064a7a124dbc86bfb2d6785bfd2274c44f1e22 \ No newline at end of file -- GitLab From 30f33052914f2915e4407e1423011713f7e0e0b2 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Tue, 30 Mar 2021 19:41:31 +0500 Subject: [PATCH 02/16] Add migrations to create trials and limits --- .../20210329102724_add_new_trail_plans.rb | 32 +++++++++++++++++-- db/schema_migrations/20210329102724 | 1 - 2 files changed, 29 insertions(+), 4 deletions(-) delete mode 100644 db/schema_migrations/20210329102724 diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index 59649a2cdf3ee6..ad420bdba3bd3b 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -6,12 +6,38 @@ class AddNewTrailPlans < ActiveRecord::Migration[6.0] DOWNTIME = false + class Plan < ActiveRecord::Base + has_one :limits, class_name: 'PlanLimits', dependent: :destroy + + def actual_limits + self.limits || self.build_limits + end + end + + class PlanLimits < ActiveRecord::Base + belongs_to :plan + end + + def create_plan_limits(plan_limit_name, plan) + plan_limit = Plan.find_or_initialize_by(name: plan_limit_name).actual_limits.dup + plan_limit.plan = plan + plan_limit.save + end + def up - execute "INSERT INTO plans (name, title, created_at, updated_at) VALUES ('premium-trial', 'Premium Trial', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" - execute "INSERT INTO plans (name, title, created_at, updated_at) VALUES ('ultimate-trial', 'Ultimate Trial', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" + return unless Gitlab.dev_env_org_or_com? || Gitlab.staging? + + ultimate_trial = Plan.create(name: 'ultimate-trial', title: 'Ultimate Trial') + premium_trial = Plan.create(name: 'premium-trial', title: 'Premium Trial') + + create_plan_limits('gold', ultimate_trial) + create_plan_limits('silver', premium_trial) end def down - execute "DELETE FROM plans WHERE name IN ('premium-trial', 'ultimate-trial')" + return unless Gitlab.dev_env_org_or_com? || Gitlab.staging? + + Plan.find_by(name: 'ultimate-trial')&.destroy + Plan.find_by(name: 'premium-trial')&.destroy end end diff --git a/db/schema_migrations/20210329102724 b/db/schema_migrations/20210329102724 deleted file mode 100644 index b2fdccf2bb878d..00000000000000 --- a/db/schema_migrations/20210329102724 +++ /dev/null @@ -1 +0,0 @@ -b40c702ea6b2120da6fe11b213064a7a124dbc86bfb2d6785bfd2274c44f1e22 \ No newline at end of file -- GitLab From 362682cc8ef0e2e49f52436291fd2996f4849b22 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Tue, 30 Mar 2021 23:02:57 +0500 Subject: [PATCH 03/16] Add changelog file with title --- changelogs/unreleased/322043-insert-plan-trial.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/322043-insert-plan-trial.yml diff --git a/changelogs/unreleased/322043-insert-plan-trial.yml b/changelogs/unreleased/322043-insert-plan-trial.yml new file mode 100644 index 00000000000000..d40a14fd8d7686 --- /dev/null +++ b/changelogs/unreleased/322043-insert-plan-trial.yml @@ -0,0 +1,5 @@ +--- +title: Add a migration fiel to insert trail plans for GL.com for Ultimate and Premium +merge_request: 57814 +author: +type: added -- GitLab From 26321cc260a403f5cb1744af49cb697e8a22f2f9 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Wed, 31 Mar 2021 14:23:58 +0500 Subject: [PATCH 04/16] Add schema migration file --- db/schema_migrations/20210329102724 | 1 + 1 file changed, 1 insertion(+) create mode 100644 db/schema_migrations/20210329102724 diff --git a/db/schema_migrations/20210329102724 b/db/schema_migrations/20210329102724 new file mode 100644 index 00000000000000..b2fdccf2bb878d --- /dev/null +++ b/db/schema_migrations/20210329102724 @@ -0,0 +1 @@ +b40c702ea6b2120da6fe11b213064a7a124dbc86bfb2d6785bfd2274c44f1e22 \ No newline at end of file -- GitLab From 04ed934d912240c9a3b8bfc0dee7fea351e6fe73 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Wed, 31 Mar 2021 15:05:12 +0500 Subject: [PATCH 05/16] Remove unused comment from migration --- db/post_migrate/20210329102724_add_new_trail_plans.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index ad420bdba3bd3b..89bd4223b0991d 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddNewTrailPlans < ActiveRecord::Migration[6.0] DOWNTIME = false -- GitLab From a1608e4327a373ca4b619dc5df5bf71038a37ade Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Wed, 31 Mar 2021 18:01:42 +0500 Subject: [PATCH 06/16] Rename trials with a _ --- db/post_migrate/20210329102724_add_new_trail_plans.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index 89bd4223b0991d..d7ba7e0539cf6e 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -24,8 +24,8 @@ def create_plan_limits(plan_limit_name, plan) def up return unless Gitlab.dev_env_org_or_com? || Gitlab.staging? - ultimate_trial = Plan.create(name: 'ultimate-trial', title: 'Ultimate Trial') - premium_trial = Plan.create(name: 'premium-trial', title: 'Premium Trial') + ultimate_trial = Plan.create(name: 'ultimate_trial', title: 'Ultimate Trial') + premium_trial = Plan.create(name: 'premium_trial', title: 'Premium Trial') create_plan_limits('gold', ultimate_trial) create_plan_limits('silver', premium_trial) @@ -34,7 +34,7 @@ def up def down return unless Gitlab.dev_env_org_or_com? || Gitlab.staging? - Plan.find_by(name: 'ultimate-trial')&.destroy - Plan.find_by(name: 'premium-trial')&.destroy + Plan.find_by(name: 'ultimate_trial')&.destroy + Plan.find_by(name: 'premium_trial')&.destroy end end -- GitLab From 8791ce57409dc5c33afd4b66870805c4b915cb6b Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Thu, 1 Apr 2021 15:17:29 +0500 Subject: [PATCH 07/16] Make review changes and use save! --- .../20210329102724_add_new_trail_plans.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index d7ba7e0539cf6e..acbe932680adeb 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -4,7 +4,7 @@ class AddNewTrailPlans < ActiveRecord::Migration[6.0] DOWNTIME = false class Plan < ActiveRecord::Base - has_one :limits, class_name: 'PlanLimits', dependent: :destroy + has_one :limits, class_name: 'PlanLimits' def actual_limits self.limits || self.build_limits @@ -18,11 +18,14 @@ class PlanLimits < ActiveRecord::Base def create_plan_limits(plan_limit_name, plan) plan_limit = Plan.find_or_initialize_by(name: plan_limit_name).actual_limits.dup plan_limit.plan = plan - plan_limit.save + plan_limit.save! end def up - return unless Gitlab.dev_env_org_or_com? || Gitlab.staging? + return unless Gitlab.dev_env_org_or_com? + + Plan.reset_column_information + PlanLimits.reset_column_information ultimate_trial = Plan.create(name: 'ultimate_trial', title: 'Ultimate Trial') premium_trial = Plan.create(name: 'premium_trial', title: 'Premium Trial') @@ -32,9 +35,8 @@ def up end def down - return unless Gitlab.dev_env_org_or_com? || Gitlab.staging? + return unless Gitlab.dev_env_org_or_com? - Plan.find_by(name: 'ultimate_trial')&.destroy - Plan.find_by(name: 'premium_trial')&.destroy + Plan.where(name: ['ultimate_trial', 'premium_trial']).delete_all end end -- GitLab From 2b6709bb0b635c7e11de8628ed423132acbc8f61 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Thu, 1 Apr 2021 15:18:50 +0500 Subject: [PATCH 08/16] Fix a typo within the changelog --- changelogs/unreleased/322043-insert-plan-trial.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/322043-insert-plan-trial.yml b/changelogs/unreleased/322043-insert-plan-trial.yml index d40a14fd8d7686..a65860a5f852b0 100644 --- a/changelogs/unreleased/322043-insert-plan-trial.yml +++ b/changelogs/unreleased/322043-insert-plan-trial.yml @@ -1,5 +1,5 @@ --- -title: Add a migration fiel to insert trail plans for GL.com for Ultimate and Premium +title: Add a migration to insert trail plans within SAAS for Ultimate and Premium plans merge_request: 57814 author: type: added -- GitLab From eff270f518085fed50c5bcee3458ef0ed3e2ece4 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Thu, 1 Apr 2021 17:55:56 +0500 Subject: [PATCH 09/16] Fix the rubocop smell --- db/post_migrate/20210329102724_add_new_trail_plans.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index acbe932680adeb..eef4d02de9b4a3 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -37,6 +37,6 @@ def up def down return unless Gitlab.dev_env_org_or_com? - Plan.where(name: ['ultimate_trial', 'premium_trial']).delete_all + Plan.where(name: %w(ultimate_trial premium_trial)).delete_all end end -- GitLab From ceec3cfa3fed6b9f8a95c579efd9a24615c8be56 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Fri, 9 Apr 2021 16:02:00 +0500 Subject: [PATCH 10/16] Add migration specs for new plans --- .../20210329102724_add_new_trail_plans.rb | 11 ++--- spec/migrations/add_new_trail_plans_spec.rb | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 spec/migrations/add_new_trail_plans_spec.rb diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index eef4d02de9b4a3..7257223401ef89 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -4,6 +4,8 @@ class AddNewTrailPlans < ActiveRecord::Migration[6.0] DOWNTIME = false class Plan < ActiveRecord::Base + self.inheritance_column = :_type_disabled + has_one :limits, class_name: 'PlanLimits' def actual_limits @@ -12,6 +14,8 @@ def actual_limits end class PlanLimits < ActiveRecord::Base + self.inheritance_column = :_type_disabled + belongs_to :plan end @@ -24,11 +28,8 @@ def create_plan_limits(plan_limit_name, plan) def up return unless Gitlab.dev_env_org_or_com? - Plan.reset_column_information - PlanLimits.reset_column_information - - ultimate_trial = Plan.create(name: 'ultimate_trial', title: 'Ultimate Trial') - premium_trial = Plan.create(name: 'premium_trial', title: 'Premium Trial') + ultimate_trial = Plan.create!(name: 'ultimate_trial', title: 'Ultimate Trial') + premium_trial = Plan.create!(name: 'premium_trial', title: 'Premium Trial') create_plan_limits('gold', ultimate_trial) create_plan_limits('silver', premium_trial) diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb new file mode 100644 index 00000000000000..a46aff8ab825c6 --- /dev/null +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +require_migration! + +RSpec.describe AddNewTrailPlans, :migration do + describe '#up' do + before do + allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true + end + + it 'creates 2 entries within the plans table' do + expect { migrate! }.to change { AddNewTrailPlans::Plan.count }.by 2 + expect(AddNewTrailPlans::Plan.last(2).pluck(:name)).to match_array(['ultimate_trial', 'premium_trial']) + end + + it 'creates 2 entries for plan limits' do + expect { migrate! }.to change { AddNewTrailPlans::PlanLimits.count }.by 2 + end + end + + describe '#down' do + before do + table(:plans).create!(name: 'random') + allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true + + end + + it 'removes the newly added ultimate and premium trial entries' do + reversible_migration do |migration| + migration.before -> { + expect(AddNewTrailPlans::Plan.count).to eql(1) + expect(AddNewTrailPlans::PlanLimits.count).to eql(0) + } + + migration.after -> { + expect(AddNewTrailPlans::Plan.count).to eql(3) + expect(AddNewTrailPlans::PlanLimits.count).to eql(2) + } + end + end + end +end -- GitLab From a12d887b954846faaacb2837a9b53d49e339d74b Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Fri, 9 Apr 2021 17:07:36 +0500 Subject: [PATCH 11/16] Fix the rubocop smells --- spec/migrations/add_new_trail_plans_spec.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb index a46aff8ab825c6..efd077be4e7b08 100644 --- a/spec/migrations/add_new_trail_plans_spec.rb +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require_migration! @@ -10,7 +12,7 @@ it 'creates 2 entries within the plans table' do expect { migrate! }.to change { AddNewTrailPlans::Plan.count }.by 2 - expect(AddNewTrailPlans::Plan.last(2).pluck(:name)).to match_array(['ultimate_trial', 'premium_trial']) + expect(AddNewTrailPlans::Plan.last(2).pluck(:name)).to match_array(%w(ultimate_trial premium_trial)) end it 'creates 2 entries for plan limits' do @@ -22,19 +24,18 @@ before do table(:plans).create!(name: 'random') allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true - end it 'removes the newly added ultimate and premium trial entries' do reversible_migration do |migration| migration.before -> { - expect(AddNewTrailPlans::Plan.count).to eql(1) - expect(AddNewTrailPlans::PlanLimits.count).to eql(0) + expect(AddNewTrailPlans::Plan.count).to be(1) + expect(AddNewTrailPlans::PlanLimits.count).to be(0) } migration.after -> { - expect(AddNewTrailPlans::Plan.count).to eql(3) - expect(AddNewTrailPlans::PlanLimits.count).to eql(2) + expect(AddNewTrailPlans::Plan.count).to be(3) + expect(AddNewTrailPlans::PlanLimits.count).to be(2) } end end -- GitLab From 929ec4a53c313354b959c3e1c9299407e01c8669 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Mon, 12 Apr 2021 14:24:06 +0500 Subject: [PATCH 12/16] Remove DOWNTIME=false as its default now --- db/post_migrate/20210329102724_add_new_trail_plans.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index 7257223401ef89..8183f143f5d068 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class AddNewTrailPlans < ActiveRecord::Migration[6.0] - DOWNTIME = false - class Plan < ActiveRecord::Base self.inheritance_column = :_type_disabled -- GitLab From 80d2276f1cf5d909ab962ef923b4ed82b158e4b2 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Mon, 12 Apr 2021 14:24:23 +0500 Subject: [PATCH 13/16] Add more specs for the checks --- spec/migrations/add_new_trail_plans_spec.rb | 93 +++++++++++++++++---- 1 file changed, 79 insertions(+), 14 deletions(-) diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb index efd077be4e7b08..52055e68cd18d1 100644 --- a/spec/migrations/add_new_trail_plans_spec.rb +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -18,25 +18,90 @@ it 'creates 2 entries for plan limits' do expect { migrate! }.to change { AddNewTrailPlans::PlanLimits.count }.by 2 end + + context 'when the plan limits for gold and silver exists' do + before do + table(:plans).create!(id: 1, name: 'gold', title: 'Gold') + table(:plan_limits).create!(id: 1, plan_id: 1, storage_size_limit: 2000) + table(:plans).create!(id: 2, name: 'silver', title: 'Silver') + table(:plan_limits).create!(id: 2, plan_id: 2, storage_size_limit: 1000) + end + + it 'duplicaes the gold and silvers plan limits entries' do + migrate! + + ultimate_plan_limits = AddNewTrailPlans::Plan.find_by(name: 'ultimate_trial').limits + expect(ultimate_plan_limits.storage_size_limit).to be 2000 + + premium_plan_limits = AddNewTrailPlans::Plan.find_by(name: 'premium_trial').limits + expect(premium_plan_limits.storage_size_limit).to be 1000 + end + end + + context 'when the instance is not SaaS' do + before do + allow(Gitlab).to receive(:dev_env_org_or_com?).and_return false + end + + it 'does not create plans and plan limits and returns' do + expect { migrate! }.not_to change { AddNewTrailPlans::Plan.count } + expect { migrate! }.not_to change { AddNewTrailPlans::Plan.count } + end + end end describe '#down' do - before do - table(:plans).create!(name: 'random') - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true + context 'when the instance is SaaS' do + before do + table(:plans).create!(name: 'random') + allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true + end + + it 'removes the newly added ultimate and premium trial entries' do + reversible_migration do |migration| + migration.before -> { + expect(AddNewTrailPlans::Plan.count).to be(1) + expect(AddNewTrailPlans::Plan.pluck(:name, :title)).to match_array( + [['random', nil]] + ) + expect(AddNewTrailPlans::PlanLimits.count).to be(0) + } + + migration.after -> { + expect(AddNewTrailPlans::Plan.pluck(:name, :title)).to match_array( + [ + ['premium_trial', 'Premium Trial'], + ['random', nil], + ['ultimate_trial', 'Ultimate Trial'] + ] + ) + expect(AddNewTrailPlans::PlanLimits.count).to be(2) + } + end + end end - it 'removes the newly added ultimate and premium trial entries' do - reversible_migration do |migration| - migration.before -> { - expect(AddNewTrailPlans::Plan.count).to be(1) - expect(AddNewTrailPlans::PlanLimits.count).to be(0) - } - - migration.after -> { - expect(AddNewTrailPlans::Plan.count).to be(3) - expect(AddNewTrailPlans::PlanLimits.count).to be(2) - } + context 'when the instance is not SaaS' do + before do + allow(Gitlab).to receive(:dev_env_org_or_com?).and_return false + table(:plans).create!(id: 1, name: 'ultimate_trial', title: 'Ultimate Trial') + table(:plans).create!(id: 2, name: 'premium_trial', title: 'Premium Trial') + table(:plan_limits).create!(id: 1, plan_id: 1) + table(:plan_limits).create!(id: 2, plan_id: 2) + end + + it 'does not delete plans and plan limits and returns' do + reversible_migration do |migration| + migration.before -> { + expect(AddNewTrailPlans::Plan.count).to be(2) + expect(AddNewTrailPlans::PlanLimits.count).to be(2) + } + + migration.after -> { + expect(AddNewTrailPlans::Plan.count).to be(2) + expect(AddNewTrailPlans::PlanLimits.count).to be(2) + } + end end end end -- GitLab From 64927fe3b72da38240a60a07f04f1a957724fa1d Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Mon, 12 Apr 2021 15:40:53 +0500 Subject: [PATCH 14/16] Use dev_env_or_com? to avoid org --- db/post_migrate/20210329102724_add_new_trail_plans.rb | 4 ++-- spec/migrations/add_new_trail_plans_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/db/post_migrate/20210329102724_add_new_trail_plans.rb b/db/post_migrate/20210329102724_add_new_trail_plans.rb index 8183f143f5d068..b142f6385f78cb 100644 --- a/db/post_migrate/20210329102724_add_new_trail_plans.rb +++ b/db/post_migrate/20210329102724_add_new_trail_plans.rb @@ -24,7 +24,7 @@ def create_plan_limits(plan_limit_name, plan) end def up - return unless Gitlab.dev_env_org_or_com? + return unless Gitlab.dev_env_or_com? ultimate_trial = Plan.create!(name: 'ultimate_trial', title: 'Ultimate Trial') premium_trial = Plan.create!(name: 'premium_trial', title: 'Premium Trial') @@ -34,7 +34,7 @@ def up end def down - return unless Gitlab.dev_env_org_or_com? + return unless Gitlab.dev_env_or_com? Plan.where(name: %w(ultimate_trial premium_trial)).delete_all end diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb index 52055e68cd18d1..8fd32d365b61d3 100644 --- a/spec/migrations/add_new_trail_plans_spec.rb +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -7,7 +7,7 @@ RSpec.describe AddNewTrailPlans, :migration do describe '#up' do before do - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true + allow(Gitlab).to receive(:dev_env_or_com?).and_return true end it 'creates 2 entries within the plans table' do @@ -40,7 +40,7 @@ context 'when the instance is not SaaS' do before do - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return false + allow(Gitlab).to receive(:dev_env_or_com?).and_return false end it 'does not create plans and plan limits and returns' do @@ -54,7 +54,7 @@ context 'when the instance is SaaS' do before do table(:plans).create!(name: 'random') - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return true + allow(Gitlab).to receive(:dev_env_or_com?).and_return true end it 'removes the newly added ultimate and premium trial entries' do @@ -83,7 +83,7 @@ context 'when the instance is not SaaS' do before do - allow(Gitlab).to receive(:dev_env_org_or_com?).and_return false + allow(Gitlab).to receive(:dev_env_or_com?).and_return false table(:plans).create!(id: 1, name: 'ultimate_trial', title: 'Ultimate Trial') table(:plans).create!(id: 2, name: 'premium_trial', title: 'Premium Trial') table(:plan_limits).create!(id: 1, plan_id: 1) -- GitLab From c02255973a0951aa0276940fffc39f0dd3cf5f94 Mon Sep 17 00:00:00 2001 From: Shreyas Agarwal Date: Tue, 13 Apr 2021 10:56:17 +0500 Subject: [PATCH 15/16] Modify specs to be more readable --- spec/migrations/add_new_trail_plans_spec.rb | 51 ++++++++------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb index 8fd32d365b61d3..e606fa118c5b5a 100644 --- a/spec/migrations/add_new_trail_plans_spec.rb +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -51,33 +51,27 @@ end describe '#down' do + before do + table(:plans).create!(id: 3, name: 'other') + table(:plan_limits).create!(plan_id: 3) + end + context 'when the instance is SaaS' do before do - table(:plans).create!(name: 'random') allow(Gitlab).to receive(:dev_env_or_com?).and_return true end it 'removes the newly added ultimate and premium trial entries' do - reversible_migration do |migration| - migration.before -> { - expect(AddNewTrailPlans::Plan.count).to be(1) - expect(AddNewTrailPlans::Plan.pluck(:name, :title)).to match_array( - [['random', nil]] - ) - expect(AddNewTrailPlans::PlanLimits.count).to be(0) - } - - migration.after -> { - expect(AddNewTrailPlans::Plan.pluck(:name, :title)).to match_array( - [ - ['premium_trial', 'Premium Trial'], - ['random', nil], - ['ultimate_trial', 'Ultimate Trial'] - ] - ) - expect(AddNewTrailPlans::PlanLimits.count).to be(2) - } - end + migrate! + + expect { described_class.new.down }.to change { AddNewTrailPlans::Plan.count }.by(-2) + expect(AddNewTrailPlans::Plan.find_by(name: 'premium_trial')).to be_nil + expect(AddNewTrailPlans::Plan.find_by(name: 'ultimate_trial')).to be_nil + + other_plan = AddNewTrailPlans::Plan.find_by(name: 'other') + expect(other_plan).to be_persisted + expect(AddNewTrailPlans::PlanLimits.count).to eq(1) + expect(AddNewTrailPlans::PlanLimits.first.plan_id).to eq(other_plan.id) end end @@ -91,17 +85,10 @@ end it 'does not delete plans and plan limits and returns' do - reversible_migration do |migration| - migration.before -> { - expect(AddNewTrailPlans::Plan.count).to be(2) - expect(AddNewTrailPlans::PlanLimits.count).to be(2) - } - - migration.after -> { - expect(AddNewTrailPlans::Plan.count).to be(2) - expect(AddNewTrailPlans::PlanLimits.count).to be(2) - } - end + migrate! + + expect { described_class.new.down }.not_to change { AddNewTrailPlans::Plan.count } + expect(AddNewTrailPlans::PlanLimits.count).to eq(3) end end end -- GitLab From 08d48d39c611b2ceba207b6511afb6ed4f1b4e56 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Tue, 13 Apr 2021 19:56:27 +0000 Subject: [PATCH 16/16] Fixes a typo on the new migration spec --- spec/migrations/add_new_trail_plans_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/migrations/add_new_trail_plans_spec.rb b/spec/migrations/add_new_trail_plans_spec.rb index e606fa118c5b5a..8ba6da11ad10df 100644 --- a/spec/migrations/add_new_trail_plans_spec.rb +++ b/spec/migrations/add_new_trail_plans_spec.rb @@ -27,7 +27,7 @@ table(:plan_limits).create!(id: 2, plan_id: 2, storage_size_limit: 1000) end - it 'duplicaes the gold and silvers plan limits entries' do + it 'duplicates the gold and silvers plan limits entries' do migrate! ultimate_plan_limits = AddNewTrailPlans::Plan.find_by(name: 'ultimate_trial').limits -- GitLab