Commit 3a0d4077 authored by Alper Akgun's avatar Alper Akgun

Merge branch 'fix-future-renewal-service' into 'master'

Fix a bug when checking future renewals for a subgroup

See merge request gitlab-org/gitlab!61139
parents 5bd0bd29 03e379f2
...@@ -10,11 +10,12 @@ ...@@ -10,11 +10,12 @@
module GitlabSubscriptions module GitlabSubscriptions
class CheckFutureRenewalService class CheckFutureRenewalService
def initialize(namespace:) def initialize(namespace:)
@namespace = namespace @namespace = namespace.root_ancestor
end end
def execute def execute
return false unless Feature.enabled?(:gitlab_subscription_future_renewal, default_enabled: :yaml) return false unless Feature.enabled?(:gitlab_subscription_future_renewal, default_enabled: :yaml)
return false unless namespace.gitlab_subscription.present?
future_renewal future_renewal
end end
......
...@@ -120,7 +120,7 @@ module Gitlab ...@@ -120,7 +120,7 @@ module Gitlab
end end
def subscription_future_renewal? def subscription_future_renewal?
return if self_managed? || namespace.nil? return if self_managed? || namespace.nil? || !namespace.gitlab_subscription.present?
::GitlabSubscriptions::CheckFutureRenewalService.new(namespace: namespace).execute ::GitlabSubscriptions::CheckFutureRenewalService.new(namespace: namespace).execute
end end
......
...@@ -112,7 +112,7 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do ...@@ -112,7 +112,7 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do
context 'with namespace' do context 'with namespace' do
let(:has_future_renewal) { false } let(:has_future_renewal) { false }
let_it_be(:namespace) { create(:group, name: 'No Limit Records') } let_it_be(:namespace) { create(:group_with_plan, name: 'No Limit Records') }
before do before do
allow_next_instance_of(GitlabSubscriptions::CheckFutureRenewalService, namespace: namespace) do |service| allow_next_instance_of(GitlabSubscriptions::CheckFutureRenewalService, namespace: namespace) do |service|
...@@ -141,6 +141,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do ...@@ -141,6 +141,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do
it { is_expected.to be_nil } it { is_expected.to be_nil }
end end
context 'without gitlab_subscription' do
let(:namespace) { build(:group, name: 'No Subscription Records') }
it 'does not check for a future renewal' do
expect(GitlabSubscriptions::CheckFutureRenewalService).not_to receive(:new)
subject
end
end
end end
end end
...@@ -200,12 +210,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do ...@@ -200,12 +210,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do
let(:has_future_renewal) { false } let(:has_future_renewal) { false }
let_it_be(:namespace) { create(:group, name: 'No Limit Records') } let_it_be(:namespace) { create(:group_with_plan, name: 'No Limit Records') }
before do before do
allow_next_instance_of(GitlabSubscriptions::CheckFutureRenewalService, namespace: namespace) do |service| allow_next_instance_of(GitlabSubscriptions::CheckFutureRenewalService, namespace: namespace) do |service|
allow(service).to receive(:execute).and_return(has_future_renewal) allow(service).to receive(:execute).and_return(has_future_renewal)
end end
allow_next_instance_of(Group) do |group|
allow(group).to receive(:gitlab_subscription).and_return(gitlab_subscription)
end
end end
where plan: %w(gold ultimate) where plan: %w(gold ultimate)
...@@ -255,6 +269,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do ...@@ -255,6 +269,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do
it { is_expected.to be_nil } it { is_expected.to be_nil }
end end
context 'without gitlab_subscription' do
let(:namespace) { build(:group, name: 'No Subscription Records') }
it 'does not check for a future renewal' do
expect(GitlabSubscriptions::CheckFutureRenewalService).not_to receive(:new)
subject
end
end
end end
end end
......
...@@ -45,6 +45,23 @@ RSpec.describe GitlabSubscriptions::CheckFutureRenewalService, :use_clean_rails_ ...@@ -45,6 +45,23 @@ RSpec.describe GitlabSubscriptions::CheckFutureRenewalService, :use_clean_rails_
end end
end end
context 'when called with a sub-group' do
let(:root_namespace) { create(:group_with_plan) }
let(:namespace) { build(:group, parent: root_namespace) }
it 'uses the root ancestor namespace' do
expect(Gitlab::SubscriptionPortal::Client).to receive(:subscription_last_term).with(root_namespace.id).and_return({})
execute_service
end
end
context 'when the namespace has no plan' do
let(:namespace) { build(:group) }
it { is_expected.to be false }
end
context 'when the `gitlab_subscription_future_renewal` feature flag is disabled' do context 'when the `gitlab_subscription_future_renewal` feature flag is disabled' do
before do before do
stub_feature_flags(gitlab_subscription_future_renewal: false) stub_feature_flags(gitlab_subscription_future_renewal: false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment