From 05c5a4b84f3b11ab4e73d2528117069b84ef168d Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 24 Mar 2020 14:48:23 +0000 Subject: [PATCH 1/7] Remove namespaces.plan_id column --- app/models/namespace.rb | 3 +++ .../20200324135315_remove_namespace_plan_id.rb | 13 +++++++++++++ db/structure.sql | 7 +------ 3 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 db/post_migrate/20200324135315_remove_namespace_plan_id.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 260ba9ea4a5839..b662aa22242d0d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -11,6 +11,9 @@ class Namespace < ApplicationRecord include FeatureGate include FromUnion include Gitlab::Utils::StrongMemoize + include IgnorableColumns + + ignore_column :plan_id, remove_with: '12.10', remove_after: '2020-05-22' # Prevent users from creating unreasonably deep level of nesting. # The number 20 was taken based on maximum nesting level of diff --git a/db/post_migrate/20200324135315_remove_namespace_plan_id.rb b/db/post_migrate/20200324135315_remove_namespace_plan_id.rb new file mode 100644 index 00000000000000..f20d3dd47f109a --- /dev/null +++ b/db/post_migrate/20200324135315_remove_namespace_plan_id.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class RemoveNamespacePlanId < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + remove_column :namespaces, :plan_id + end + + def down + add_column :namespaces, :plan_id, :integer + end +end diff --git a/db/structure.sql b/db/structure.sql index 90585a157cfad3..43e268becdfbec 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4005,7 +4005,6 @@ CREATE TABLE public.namespaces ( require_two_factor_authentication boolean DEFAULT false NOT NULL, two_factor_grace_period integer DEFAULT 48 NOT NULL, cached_markdown_version integer, - plan_id integer, project_creation_level integer, runners_token character varying, trial_ends_on timestamp with time zone, @@ -9621,8 +9620,6 @@ CREATE INDEX index_namespaces_on_path ON public.namespaces USING btree (path); CREATE INDEX index_namespaces_on_path_trigram ON public.namespaces USING gin (path public.gin_trgm_ops); -CREATE INDEX index_namespaces_on_plan_id ON public.namespaces USING btree (plan_id); - CREATE INDEX index_namespaces_on_require_two_factor_authentication ON public.namespaces USING btree (require_two_factor_authentication); CREATE UNIQUE INDEX index_namespaces_on_runners_token ON public.namespaces USING btree (runners_token); @@ -10975,9 +10972,6 @@ ALTER TABLE ONLY public.system_note_metadata ALTER TABLE ONLY public.merge_requests ADD CONSTRAINT fk_fd82eae0b9 FOREIGN KEY (head_pipeline_id) REFERENCES public.ci_pipelines(id) ON DELETE SET NULL; -ALTER TABLE ONLY public.namespaces - ADD CONSTRAINT fk_fdd12e5b80 FOREIGN KEY (plan_id) REFERENCES public.plans(id) ON DELETE SET NULL; - ALTER TABLE ONLY public.project_import_data ADD CONSTRAINT fk_ffb9ee3a10 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE; @@ -13061,6 +13055,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200323134519 20200324093258 20200324115359 +20200324135315 20200325152327 20200325160952 20200325183636 -- GitLab From d0971e7e3d934cd129b8956668ea817279ca09ed Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 26 Mar 2020 16:59:03 +0000 Subject: [PATCH 2/7] Add {group,namespace}_with_plan factories --- ee/spec/factories/namespaces.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/ee/spec/factories/namespaces.rb b/ee/spec/factories/namespaces.rb index c38ebd3a1f0646..de9a5b953bcbb0 100644 --- a/ee/spec/factories/namespaces.rb +++ b/ee/spec/factories/namespaces.rb @@ -40,3 +40,22 @@ end end end + +FactoryBot.define do + factory :namespace_with_plan, parent: :namespace do + transient do + plan { :default_plan } + trial_ends_on { nil } + end + + after(:create) do |namespace, evaluator| + if evaluator.plan + create(:gitlab_subscription, + namespace: namespace, + hosted_plan: create(evaluator.plan), + trial: evaluator.trial_ends_on.present?, + trial_ends_on: evaluator.trial_ends_on) + end + end + end +end -- GitLab From 43e62f61c88554717bc8e6c0f01b9ca289f67867 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 30 Mar 2020 16:12:29 +0100 Subject: [PATCH 3/7] Fix plan creation in dashboard specs --- .../features/gold_trial_callout_shared_examples.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/spec/support/shared_examples/features/gold_trial_callout_shared_examples.rb b/ee/spec/support/shared_examples/features/gold_trial_callout_shared_examples.rb index 90a6868441867c..e8f704936af523 100644 --- a/ee/spec/support/shared_examples/features/gold_trial_callout_shared_examples.rb +++ b/ee/spec/support/shared_examples/features/gold_trial_callout_shared_examples.rb @@ -49,7 +49,6 @@ it 'hides promotion callout if a gold plan is active', :js do group = create(:group_with_plan, name: 'gold group', plan: :gold_plan) - group.update(plan: group.gitlab_subscription.hosted_plan) group.add_owner(user) visit page_path -- GitLab From fe4c86fc84c6a8de4018ce064e29e27cd399bd6d Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Tue, 31 Mar 2020 14:37:18 +0800 Subject: [PATCH 4/7] Remove namespace association in plans --- config/pseudonymizer.yml | 1 - ee/app/models/plan.rb | 1 - ee/spec/models/plan_spec.rb | 4 ---- 3 files changed, 6 deletions(-) diff --git a/config/pseudonymizer.yml b/config/pseudonymizer.yml index 7b5f8aad25569c..195506ac4a150b 100644 --- a/config/pseudonymizer.yml +++ b/config/pseudonymizer.yml @@ -239,7 +239,6 @@ tables: - repository_size_limit - require_two_factor_authentication - two_factor_grace_period - - plan_id - project_creation_level members: whitelist: diff --git a/ee/app/models/plan.rb b/ee/app/models/plan.rb index 8af4fd94469311..79b16a83a8f9cf 100644 --- a/ee/app/models/plan.rb +++ b/ee/app/models/plan.rb @@ -13,7 +13,6 @@ class Plan < ApplicationRecord DEFAULT_PLANS = [DEFAULT, FREE].freeze ALL_HOSTED_PLANS = (PAID_HOSTED_PLANS + [EARLY_ADOPTER]).freeze - has_many :namespaces has_many :hosted_subscriptions, class_name: 'GitlabSubscription', foreign_key: 'hosted_plan_id' has_one :limits, class_name: 'PlanLimits' diff --git a/ee/spec/models/plan_spec.rb b/ee/spec/models/plan_spec.rb index a4bc6cb2d65852..7a46760bddc213 100644 --- a/ee/spec/models/plan_spec.rb +++ b/ee/spec/models/plan_spec.rb @@ -3,10 +3,6 @@ require 'spec_helper' describe Plan do - describe 'associations' do - it { is_expected.to have_many(:namespaces) } - end - describe '#default?' do subject { plan.default? } -- GitLab From b8b9f876260ec80de517f0b293f0d077f222a5c9 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Tue, 31 Mar 2020 15:25:30 +0800 Subject: [PATCH 5/7] Remove plan relationship in namespaces Also removes validations and scopes related to it --- ee/app/models/ee/namespace.rb | 20 -------------------- ee/spec/models/ee/namespace_spec.rb | 28 ---------------------------- 2 files changed, 48 deletions(-) diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 7f70685287dffa..23818cc3ab8c7a 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -27,8 +27,6 @@ module Namespace attr_writer :root_ancestor - belongs_to :plan - has_one :namespace_statistics has_one :gitlab_subscription, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -62,7 +60,6 @@ module Namespace # Opportunistically clear the +file_template_project_id+ if invalid before_validation :clear_file_template_project_id - validate :validate_plan_name validate :validate_shared_runner_minutes_support validates :max_pages_size, @@ -293,17 +290,6 @@ def shared_runners_enabled? all_projects.with_shared_runners.any? end - # These helper methods are required to not break the Namespace API. - def plan=(plan_name) - if plan_name.is_a?(String) - @plan_name = plan_name # rubocop:disable Gitlab/ModuleWithInstanceVariables - - super(Plan.find_by(name: @plan_name)) # rubocop:disable Gitlab/ModuleWithInstanceVariables - else - super - end - end - def memoized_plans=(plans) @plans = plans # rubocop: disable Gitlab/ModuleWithInstanceVariables end @@ -396,12 +382,6 @@ def use_elasticsearch? private - def validate_plan_name - if defined?(@plan_name) && @plan_name.present? && PLANS.exclude?(@plan_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables - errors.add(:plan, 'is not included in the list') - end - end - def validate_shared_runner_minutes_support return if shared_runner_minutes_supported? diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index d86c1a6dec1ce6..5f58c5178d368b 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -14,7 +14,6 @@ it { is_expected.to have_one(:namespace_statistics) } it { is_expected.to have_one(:gitlab_subscription).dependent(:destroy) } - it { is_expected.to belong_to(:plan) } it { is_expected.to delegate_method(:extra_shared_runners_minutes).to(:namespace_statistics) } it { is_expected.to delegate_method(:shared_runners_minutes).to(:namespace_statistics) } @@ -262,33 +261,6 @@ end describe 'custom validations' do - describe '#validate_plan_name' do - let(:group) { build(:group) } - - context 'with a valid plan name' do - it 'is valid' do - group.plan = create(:bronze_plan) - - expect(group).to be_valid - end - end - - context 'with an invalid plan name' do - it 'is invalid when `unknown`' do - group.plan = 'unknown' - - expect(group).not_to be_valid - expect(group.errors[:plan]).to include('is not included in the list') - end - - it 'is valid for blank strings' do - group.plan = ' ' - - expect(group).to be_valid - end - end - end - describe '#validate_shared_runner_minutes_support' do context 'when changing :shared_runners_minutes_limit' do before do -- GitLab From ad7cd32ea6db00f1be7d6601305fec616fb10871 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Tue, 31 Mar 2020 16:53:55 +0800 Subject: [PATCH 6/7] Fix various spec failures Makes SystemHookService use the persisted subscription to make tests pass for now --- ee/app/models/ee/pages_domain.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/models/ee/pages_domain.rb b/ee/app/models/ee/pages_domain.rb index 564d9daa725086..c2824faeda3e89 100644 --- a/ee/app/models/ee/pages_domain.rb +++ b/ee/app/models/ee/pages_domain.rb @@ -5,7 +5,7 @@ module PagesDomain extend ActiveSupport::Concern prepended do - scope :with_logging_info, -> { includes(project: [:route, { namespace: [:plan, :gitlab_subscription] }]) } + scope :with_logging_info, -> { includes(project: [:route, { namespace: [:gitlab_subscription] }]) } end end end -- GitLab From b5faed7989ce65af9f8f48338f9ebf201f815eac Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Mon, 6 Apr 2020 11:31:14 +0800 Subject: [PATCH 7/7] Clean up specs --- ee/spec/factories/namespaces.rb | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/ee/spec/factories/namespaces.rb b/ee/spec/factories/namespaces.rb index de9a5b953bcbb0..c38ebd3a1f0646 100644 --- a/ee/spec/factories/namespaces.rb +++ b/ee/spec/factories/namespaces.rb @@ -40,22 +40,3 @@ end end end - -FactoryBot.define do - factory :namespace_with_plan, parent: :namespace do - transient do - plan { :default_plan } - trial_ends_on { nil } - end - - after(:create) do |namespace, evaluator| - if evaluator.plan - create(:gitlab_subscription, - namespace: namespace, - hosted_plan: create(evaluator.plan), - trial: evaluator.trial_ends_on.present?, - trial_ends_on: evaluator.trial_ends_on) - end - end - end -end -- GitLab