diff --git a/app/models/plan_limits.rb b/app/models/plan_limits.rb index e6019e585795675d0cfba2d6256e2b9152cb84d3..56298a4f1ca7a3f858e2cb6e738000f411d0fa36 100644 --- a/app/models/plan_limits.rb +++ b/app/models/plan_limits.rb @@ -17,6 +17,9 @@ class PlanLimits < ApplicationRecord belongs_to :plan + before_validation :set_plan_name_uid + + validates :plan_name_uid, presence: true, if: :plan_id? validates :notification_limit, numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :enforcement_limit, numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :web_hook_calls, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, @@ -78,6 +81,12 @@ def format_limits_history(user, new_limits) limits_history end + + def set_plan_name_uid + return if plan_name_uid.present? && !plan_id_changed? + + self.plan_name_uid = plan&.plan_name_uid_before_type_cast + end end PlanLimits.prepend_mod_with('PlanLimits') diff --git a/ee/app/models/ee/ci/pending_build.rb b/ee/app/models/ee/ci/pending_build.rb index 0056c90a7ad43f5004715c000aff66e02225f0f5..57456b813d1e784d22bd4c8f2e2f95896fb83fb7 100644 --- a/ee/app/models/ee/ci/pending_build.rb +++ b/ee/app/models/ee/ci/pending_build.rb @@ -17,8 +17,12 @@ module PendingBuild override :args_from_build def args_from_build(build) fields = { minutes_exceeded: minutes_exceeded?(build.project) } - fields[:plan_id] = build.project.actual_plan.id if - ::Gitlab::Saas.feature_available?(:ci_runners_allowed_plans) + + if ::Gitlab::Saas.feature_available?(:ci_runners_allowed_plans) + plan = build.project.actual_plan + fields[:plan_id] = plan.id + fields[:plan_name_uid] = plan.plan_name_uid_before_type_cast + end super.merge(fields) end diff --git a/ee/app/models/ee/ci/runner.rb b/ee/app/models/ee/ci/runner.rb index c1c330c586989e3d88c8596adccd824bbbc295f8..84d0fecf776fa51bc1a0506007f653d6beee6721 100644 --- a/ee/app/models/ee/ci/runner.rb +++ b/ee/app/models/ee/ci/runner.rb @@ -9,6 +9,10 @@ module Runner MOST_ACTIVE_RUNNERS_BUILDS_LIMIT = 1000 prepended do + before_validation :set_allowed_plan_name_uids + + validates :allowed_plan_name_uids, presence: true, if: -> { allowed_plan_ids.present? } + has_one :cost_settings, class_name: 'Ci::Minutes::CostSetting', foreign_key: :runner_id, inverse_of: :runner has_many :hosted_runner_monthly_usages, class_name: 'Ci::Minutes::GitlabHostedRunnerMonthlyUsage', @@ -78,6 +82,12 @@ def cost_factor end end + def set_allowed_plan_name_uids + return if allowed_plan_name_uids.present? && !allowed_plan_ids_changed? + + self.allowed_plan_name_uids = ::Plan.where(id: allowed_plan_ids).map(&:plan_name_uid_before_type_cast) + end + class_methods do def most_active_runners(inner_query_fn = nil) inner_query = ::Ci::RunningBuild.select( diff --git a/ee/app/models/gitlab_subscription.rb b/ee/app/models/gitlab_subscription.rb index f9a66fd6a6027d08c00d97dc53ea18198dc9d99d..d4a9346ab69ba2c91c90ccb3b41cb41a7b53ea0c 100644 --- a/ee/app/models/gitlab_subscription.rb +++ b/ee/app/models/gitlab_subscription.rb @@ -12,6 +12,8 @@ class GitlabSubscription < ApplicationRecord attribute :start_date, default: -> { Date.today } + before_validation :set_hosted_plan_name_uid + before_update :set_max_seats_used_changed_at before_update :log_previous_state_for_update, if: :tracked_attributes_changed? before_update :reset_seat_statistics @@ -29,6 +31,7 @@ class GitlabSubscription < ApplicationRecord validates :trial_ends_on, :trial_starts_on, presence: true, if: :trial? validates_comparison_of :trial_ends_on, greater_than: :trial_starts_on, if: :trial? + validates :hosted_plan_name_uid, presence: true, if: :hosted_plan_id? delegate :name, :title, to: :hosted_plan, prefix: :plan, allow_nil: true delegate :exclude_guests?, to: :namespace @@ -258,6 +261,12 @@ def reset_seats_usage_callouts Groups::ResetSeatCalloutsWorker.perform_async(namespace_id) end + + def set_hosted_plan_name_uid + return if hosted_plan_name_uid.present? && !hosted_plan_id_changed? + + self.hosted_plan_name_uid = hosted_plan&.plan_name_uid_before_type_cast + end end # Added for JiHu diff --git a/ee/spec/models/ee/ci/pending_build_spec.rb b/ee/spec/models/ee/ci/pending_build_spec.rb index fb810da45770702c228c4d8084970fcd20cd4ed3..8c1e2e7ed5902d59a96a692d7c45e6afe8a2ea81 100644 --- a/ee/spec/models/ee/ci/pending_build_spec.rb +++ b/ee/spec/models/ee/ci/pending_build_spec.rb @@ -110,6 +110,14 @@ expect(described_class.last.plan_id).to eq(premium_plan.id) end + + it 'sets plan_name_uid alongside plan_id' do + expect { described_class.upsert_from_build!(build) }.to change(described_class, :count).by(1) + + pending_build = described_class.last + expect(pending_build.plan_id).to eq(premium_plan.id) + expect(pending_build.plan_name_uid).to eq(Plan::PLAN_NAME_UID_LIST[:premium]) + end end context 'when not on SaaS' do @@ -118,6 +126,12 @@ expect(described_class.last.plan_id).to be_nil end + + it 'plan_name_uid is also empty' do + expect { described_class.upsert_from_build!(build) }.to change(described_class, :count).by(1) + + expect(described_class.last.plan_name_uid).to be_nil + end end end end diff --git a/ee/spec/models/ee/ci/runner_spec.rb b/ee/spec/models/ee/ci/runner_spec.rb index a234217c814b180844f921654431021669c4e48b..6ea5483b75e8c54d516c47e57c66f22b51717663 100644 --- a/ee/spec/models/ee/ci/runner_spec.rb +++ b/ee/spec/models/ee/ci/runner_spec.rb @@ -359,5 +359,111 @@ it { is_expected.to include(premium_plan.name) } end end + + describe '#set_allowed_plan_name_uids' do + let_it_be(:free_plan) { create(:free_plan) } + let_it_be(:premium_plan) { create(:premium_plan) } + let_it_be(:ultimate_plan) { create(:ultimate_plan) } + + context 'on create' do + it 'sets allowed_plan_name_uids from allowed_plan_ids' do + runner = build(:ci_runner, :instance, allowed_plan_ids: [premium_plan.id, ultimate_plan.id]) + + expect { runner.save! } + .to change { runner.allowed_plan_name_uids } + .from([]) + + expect(runner.allowed_plan_name_uids).to match_array([ + Plan::PLAN_NAME_UID_LIST[:premium], + Plan::PLAN_NAME_UID_LIST[:ultimate] + ]) + end + + it 'sets UIDs for multiple plans' do + runner = create(:ci_runner, :instance, allowed_plan_ids: [ultimate_plan.id, free_plan.id, premium_plan.id]) + + expect(runner.allowed_plan_name_uids).to match_array([ + Plan::PLAN_NAME_UID_LIST[:ultimate], + Plan::PLAN_NAME_UID_LIST[:free], + Plan::PLAN_NAME_UID_LIST[:premium] + ]) + end + end + + context 'on update' do + it 'updates allowed_plan_name_uids when allowed_plan_ids changes' do + runner = create(:ci_runner, :instance, allowed_plan_ids: [premium_plan.id]) + + expect { runner.update!(allowed_plan_ids: [ultimate_plan.id]) } + .to change { runner.reload.allowed_plan_name_uids } + .from([Plan::PLAN_NAME_UID_LIST[:premium]]) + .to([Plan::PLAN_NAME_UID_LIST[:ultimate]]) + end + + it 'does not change allowed_plan_name_uids when allowed_plan_ids stays the same' do + runner = create(:ci_runner, :instance, allowed_plan_ids: [premium_plan.id]) + original_uids = runner.allowed_plan_name_uids + + runner.update!(description: 'Updated description') + + expect(runner.reload.allowed_plan_name_uids).to eq(original_uids) + expect(original_uids).to match_array([Plan::PLAN_NAME_UID_LIST[:premium]]) + end + end + + context 'when using allowed_plans= setter' do + it 'sets both allowed_plan_ids and allowed_plan_name_uids' do + runner = build(:ci_runner, :instance) + runner.allowed_plans = %w[premium ultimate] + + runner.save! + + expect(runner.allowed_plan_ids).to match_array([premium_plan.id, ultimate_plan.id]) + expect(runner.allowed_plan_name_uids).to match_array([ + Plan::PLAN_NAME_UID_LIST[:premium], + Plan::PLAN_NAME_UID_LIST[:ultimate] + ]) + end + end + + context 'when allowed_plan_ids is empty' do + it 'keeps allowed_plan_name_uids empty' do + runner = create(:ci_runner, :instance, allowed_plan_ids: []) + + expect(runner.allowed_plan_name_uids).to eq([]) + end + end + + context 'when allowed_plan_ids is cleared' do + it 'clears allowed_plan_name_uids' do + runner = create(:ci_runner, :instance, allowed_plan_ids: [premium_plan.id]) + + expect { runner.update!(allowed_plan_ids: []) } + .to change { runner.reload.allowed_plan_name_uids } + .from([Plan::PLAN_NAME_UID_LIST[:premium]]) + .to([]) + end + end + + context 'when a plan ID does not exist' do + it 'only includes UIDs for existing plans' do + non_existent_id = Plan.maximum(:id).to_i + 1 + runner = create(:ci_runner, :instance, allowed_plan_ids: [premium_plan.id, non_existent_id]) + + expect(runner.allowed_plan_name_uids).to match_array([Plan::PLAN_NAME_UID_LIST[:premium]]) + end + end + + context 'when allowed_plan_name_uids is missing' do + it 'populates allowed_plan_name_uids on save' do + runner = create(:ci_runner, :instance, allowed_plan_ids: [premium_plan.id]) + runner.update_column(:allowed_plan_name_uids, []) + + runner.reload.update!(description: 'Updated') + + expect(runner.reload.allowed_plan_name_uids).to match_array([Plan::PLAN_NAME_UID_LIST[:premium]]) + end + end + end end end diff --git a/ee/spec/models/gitlab_subscription_spec.rb b/ee/spec/models/gitlab_subscription_spec.rb index acc27ef29e0de3dc2aa6de265a8c01d7d7d25ae0..da23b59c592b8836673b64ede371ae4511bdaae6 100644 --- a/ee/spec/models/gitlab_subscription_spec.rb +++ b/ee/spec/models/gitlab_subscription_spec.rb @@ -1126,6 +1126,76 @@ end end + describe '#set_hosted_plan_name_uid' do + let_it_be(:namespace) { create(:namespace) } + let(:premium_plan) { create(:premium_plan) } + let(:ultimate_plan) { create(:ultimate_plan) } + + context 'on create' do + it 'sets hosted_plan_name_uid from the associated hosted_plan' do + subscription = build(:gitlab_subscription, namespace: namespace, hosted_plan: premium_plan) + + expect { subscription.save! } + .to change { subscription.hosted_plan_name_uid } + .from(nil).to(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + + context 'on update' do + it 'updates hosted_plan_name_uid when hosted_plan changes' do + subscription = create(:gitlab_subscription, namespace: namespace, hosted_plan: premium_plan) + + expect { subscription.update!(hosted_plan: ultimate_plan) } + .to change { subscription.reload.hosted_plan_name_uid } + .from(Plan::PLAN_NAME_UID_LIST[:premium]) + .to(Plan::PLAN_NAME_UID_LIST[:ultimate]) + end + + it 'does not change hosted_plan_name_uid when hosted_plan stays the same' do + subscription = create(:gitlab_subscription, namespace: namespace, hosted_plan: premium_plan) + original_uid = subscription.hosted_plan_name_uid + + subscription.update!(seats: 50) + + expect(subscription.reload.hosted_plan_name_uid).to eq(original_uid) + expect(original_uid).to eq(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + + context 'when using plan_code= setter' do + it 'sets hosted_plan_name_uid via the setter' do + subscription = build(:gitlab_subscription, namespace: namespace) + subscription.plan_code = 'premium' + + expect { subscription.save! } + .to change { subscription.hosted_plan_name_uid } + .from(nil).to(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + + context 'when hosted_plan is set to nil' do + it 'clears hosted_plan_name_uid' do + subscription = create(:gitlab_subscription, namespace: namespace, hosted_plan: premium_plan) + + expect { subscription.update!(hosted_plan: nil) } + .to change { subscription.reload.hosted_plan_name_uid } + .from(Plan::PLAN_NAME_UID_LIST[:premium]) + .to(nil) + end + end + + context 'when hosted_plan_name_uid is missing' do + it 'populates hosted_plan_name_uid on save' do + subscription = create(:gitlab_subscription, namespace: namespace, hosted_plan: premium_plan) + subscription.update_column(:hosted_plan_name_uid, nil) + + subscription.reload.update!(seats: 100) + + expect(subscription.reload.hosted_plan_name_uid).to eq(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + end + describe 'constants' do it 'defines SERVICE_ACCOUNT_LIMIT_FOR_TRIAL' do expect(described_class::SERVICE_ACCOUNT_LIMIT_FOR_TRIAL).to eq(100) diff --git a/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb b/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb index 68a70387e3b30b5cf390ab7ff1bd56f7f452913f..19cff7df6264b18d858d70a55dbd9d0191138d26 100644 --- a/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb +++ b/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb @@ -203,5 +203,30 @@ ) end end + + context 'when tracking hosted_plan_name_uid' do + let_it_be(:premium_plan) { create(:premium_plan) } + let_it_be(:ultimate_plan) { create(:ultimate_plan) } + + it 'captures hosted_plan_name_uid in the history record' do + record = described_class.create_from_change( + :gitlab_subscription_updated, + { + 'id' => 1, + 'namespace_id' => group1.id, + 'hosted_plan_id' => premium_plan.id, + 'hosted_plan_name_uid' => Plan::PLAN_NAME_UID_LIST[:premium], + 'seats' => 10 + } + ) + + expect(record).to be_persisted + expect(record.hosted_plan_name_uid).to eq(Plan::PLAN_NAME_UID_LIST[:premium]) + end + + it 'includes hosted_plan_name_uid in TRACKED_ATTRIBUTES' do + expect(GitlabSubscriptions::SubscriptionHistory::TRACKED_ATTRIBUTES).to include('hosted_plan_name_uid') + end + end end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 7e724dedc00bf24078053039f9ce15aa58a673a8..e71ba0717403cac6d046d62c284ac1554912d4c0 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1313,15 +1313,22 @@ def expect_db_update end context 'deduplicates on allowed_plan_ids' do + let_it_be(:premium_plan) { create(:premium_plan) } + let_it_be(:ultimate_plan) { create(:ultimate_plan) } + let_it_be(:free_plan) { create(:free_plan) } + let_it_be(:bronze_plan) { create(:bronze_plan) } + before do - create_list(:ci_runner, 2, allowed_plan_ids: [1, 2]) - create_list(:ci_runner, 2, allowed_plan_ids: [3, 4]) + create_list(:ci_runner, 2, allowed_plan_ids: [premium_plan.id, ultimate_plan.id]) + create_list(:ci_runner, 2, allowed_plan_ids: [free_plan.id, bronze_plan.id]) end it 'creates two matchers' do expect(matchers.size).to eq(2) - expect(matchers.map(&:allowed_plan_ids)).to match_array([[1, 2], [3, 4]]) + expect(matchers.map(&:allowed_plan_ids)).to match_array( + [[premium_plan.id, ultimate_plan.id], [free_plan.id, bronze_plan.id]] + ) end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index 0ac6287adbfc43f868dac7855f27fd34e4bdc5ed..022fb468f9fc3b24fa284a70059c40ee23babc5f 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -279,7 +279,7 @@ end let(:columns_with_nil) do - %w[repository_size plan_name_uid] + %w[repository_size] end let(:datetime_columns) do @@ -323,6 +323,68 @@ end end + describe '#set_plan_name_uid' do + let(:premium_plan) { create(:plan, name: 'premium') } + let(:ultimate_plan) { create(:plan, name: 'ultimate') } + + context 'on create' do + it 'sets plan_name_uid from the associated plan' do + plan_limits = build(:plan_limits, plan: premium_plan) + + expect { plan_limits.save! } + .to change { plan_limits.plan_name_uid } + .from(nil).to(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + + context 'on update' do + it 'updates plan_name_uid when plan changes' do + plan_limits = create(:plan_limits, plan: premium_plan) + + expect { plan_limits.update!(plan: ultimate_plan) } + .to change { plan_limits.reload.plan_name_uid } + .from(Plan::PLAN_NAME_UID_LIST[:premium]) + .to(Plan::PLAN_NAME_UID_LIST[:ultimate]) + end + + it 'does not change plan_name_uid when plan stays the same' do + plan_limits = create(:plan_limits, plan: premium_plan) + original_uid = plan_limits.plan_name_uid + + plan_limits.update!(ci_pipeline_size: 100) + + expect(plan_limits.reload.plan_name_uid).to eq(original_uid) + expect(original_uid).to eq(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + + context 'when plan is set to nil' do + it 'sets plan_name_uid to nil' do + plan_limits = build(:plan_limits, plan: premium_plan) + plan_limits.save! + + plan_limits.plan = nil + + # The callback should handle nil gracefully and set plan_name_uid to nil + # Note: the database has a NOT NULL constraint on plan_id, so this scenario + # can only occur in-memory before validation + expect { plan_limits.send(:set_plan_name_uid) }.not_to raise_error + expect(plan_limits.plan_name_uid).to be_nil + end + end + + context 'when plan_name_uid is missing' do + it 'populates plan_name_uid on save' do + plan_limits = create(:plan_limits, plan: premium_plan) + plan_limits.update_column(:plan_name_uid, nil) + + plan_limits.reload.update!(ci_pipeline_size: 200) + + expect(plan_limits.reload.plan_name_uid).to eq(Plan::PLAN_NAME_UID_LIST[:premium]) + end + end + end + describe '#format_limits_history', :freeze_time do let(:user) { create(:user) } let(:plan_limits) { create(:plan_limits, plan: create(:plan, name: 'premium')) }