diff --git a/ee/app/models/gitlab_subscription.rb b/ee/app/models/gitlab_subscription.rb index 13c5566890280a200705ffee9efb98323037fd48..f9f1f2c28379a918174d5c5123ced6cbe54d5848 100644 --- a/ee/app/models/gitlab_subscription.rb +++ b/ee/app/models/gitlab_subscription.rb @@ -9,7 +9,10 @@ class GitlabSubscription < ApplicationRecord enum trial_extension_type: { extended: 1, reactivated: 2 } default_value_for(:start_date) { Date.today } + before_update :log_previous_state_for_update + before_update :reset_seats_for_new_term + after_commit :index_namespace, on: [:create, :update] after_destroy_commit :log_previous_state_for_destroy @@ -68,9 +71,9 @@ def calculate_seats_owed end # Refresh seat related attribute (without persisting them) - def refresh_seat_attributes! + def refresh_seat_attributes!(new_term_reset: false) self.seats_in_use = calculate_seats_in_use - self.max_seats_used = [max_seats_used, seats_in_use].max + self.max_seats_used = new_term_reset ? seats_in_use : [max_seats_used, seats_in_use].max self.seats_owed = calculate_seats_owed end @@ -175,4 +178,17 @@ def index_namespace ElasticsearchIndexedNamespace.safe_find_or_create_by!(namespace_id: namespace_id) end + + # If the subscription starts a new term, the dates will change. We reset seat counts + # so that we don't carry over max_seats_used/owed from the previous term + def reset_seats_for_new_term + return unless new_term? + + refresh_seat_attributes!(new_term_reset: true) + end + + def new_term? + persisted? && start_date_changed? && end_date_changed? && + (end_date_was.nil? || start_date >= end_date_was) + end end diff --git a/ee/spec/models/gitlab_subscription_spec.rb b/ee/spec/models/gitlab_subscription_spec.rb index d18b2101195b3fca3a3b32a3666faa57cee1e719..1506c9aea923284878e2d188d6aac36704532ca1 100644 --- a/ee/spec/models/gitlab_subscription_spec.rb +++ b/ee/spec/models/gitlab_subscription_spec.rb @@ -163,7 +163,7 @@ end describe '#refresh_seat_attributes!' do - subject { create(:gitlab_subscription, seats: 3, max_seats_used: 2) } + subject(:gitlab_subscription) { create(:gitlab_subscription, seats: 3, max_seats_used: 2) } before do expect(subject).to receive(:calculate_seats_in_use).and_return(calculate_seats_in_use) @@ -174,10 +174,20 @@ it 'does not increase max_seats_used' do expect do - subject.refresh_seat_attributes! - end.to change(subject, :seats_in_use).from(0).to(1) - .and not_change(subject, :max_seats_used) - .and not_change(subject, :seats_owed) + gitlab_subscription.refresh_seat_attributes! + end.to change(gitlab_subscription, :seats_in_use).from(0).to(1) + .and not_change(gitlab_subscription, :max_seats_used) + .and not_change(gitlab_subscription, :seats_owed) + end + + context 'when resetting for a new term' do + it 'sets max_seats_used to seats_in_use' do + expect do + gitlab_subscription.refresh_seat_attributes!(new_term_reset: true) + end.to change(gitlab_subscription, :seats_in_use).from(0).to(1) + .and change(gitlab_subscription, :max_seats_used).from(2).to(1) + .and not_change(gitlab_subscription, :seats_owed) + end end end @@ -186,10 +196,20 @@ it 'increases seats and max_seats_used' do expect do - subject.refresh_seat_attributes! - end.to change(subject, :seats_in_use).from(0).to(4) - .and change(subject, :max_seats_used).from(2).to(4) - .and change(subject, :seats_owed).from(0).to(1) + gitlab_subscription.refresh_seat_attributes! + end.to change(gitlab_subscription, :seats_in_use).from(0).to(4) + .and change(gitlab_subscription, :max_seats_used).from(2).to(4) + .and change(gitlab_subscription, :seats_owed).from(0).to(1) + end + + context 'when resetting for a new term' do + it 'sets max_seats_used to seats_in_use' do + expect do + gitlab_subscription.refresh_seat_attributes!(new_term_reset: true) + end.to change(gitlab_subscription, :seats_in_use).from(0).to(4) + .and change(gitlab_subscription, :max_seats_used).from(2).to(4) + .and change(gitlab_subscription, :seats_owed).from(0).to(1) + end end end end @@ -432,11 +452,13 @@ expect(GitlabSubscriptionHistory.attribute_names - described_class.attribute_names).to eq(diff_attrs) end - context 'before_update' do - it 'logs previous state to gitlab subscription history' do - gitlab_subscription = create(:gitlab_subscription, max_seats_used: 42, seats: 13, namespace: create(:namespace)) + context 'before_update', :freeze_time do + let(:gitlab_subscription) do + create(:gitlab_subscription, max_seats_used: 42, seats: 13, seats_owed: 29, start_date: Date.today - 1.year) + end - gitlab_subscription.update! max_seats_used: 32 + it 'logs previous state to gitlab subscription history' do + gitlab_subscription.update!(max_seats_used: 32) expect(GitlabSubscriptionHistory.count).to eq(1) expect(GitlabSubscriptionHistory.last.attributes).to include( @@ -446,6 +468,75 @@ 'seats' => 13 ) end + + context 'when start and end dates change' do + before do + allow(gitlab_subscription).to receive(:seats_in_use).and_return(20) + end + + context 'when start_date is after the old end_date' do + it 'resets seats attributes' do + new_start = gitlab_subscription.end_date + 1.year + new_end = new_start + 1.year + + expect do + gitlab_subscription.update!(start_date: new_start, end_date: new_end) + end.to change(gitlab_subscription, :start_date).to(new_start) + .and change(gitlab_subscription, :end_date).to(new_end) + .and change(gitlab_subscription, :max_seats_used).from(42).to(20) + .and change(gitlab_subscription, :seats_owed).from(29).to(7) + end + end + + context 'when the end_date was nil' do + it 'resets seats attributes' do + gitlab_subscription.update!(end_date: nil) + + new_start = Date.today + 1.year + new_end = new_start + 1.year + + expect do + gitlab_subscription.update!(start_date: new_start, end_date: new_end) + end.to change(gitlab_subscription, :start_date).to(new_start) + .and change(gitlab_subscription, :end_date).to(new_end) + .and change(gitlab_subscription, :max_seats_used).from(42).to(20) + .and change(gitlab_subscription, :seats_owed).from(29).to(7) + end + end + + context 'when start_date is not after the old end_date' do + it 'does not reset seats attributes' do + new_start = gitlab_subscription.end_date - 1.month + new_end = new_start + 1.year + + expect do + gitlab_subscription.update!(start_date: new_start, end_date: new_end) + end.to change(gitlab_subscription, :start_date).to(new_start) + .and change(gitlab_subscription, :end_date).to(new_end) + .and not_change(gitlab_subscription, :max_seats_used) + .and not_change(gitlab_subscription, :seats_owed) + end + end + end + + context 'when only one date is changed' do + it 'does not reset seats attributes' do + expect do + gitlab_subscription.update!(start_date: Date.today) + end.to change(gitlab_subscription, :start_date).to(Date.today) + .and not_change(gitlab_subscription, :max_seats_used) + .and not_change(gitlab_subscription, :seats_owed) + end + end + + context 'when no dates are changed' do + it 'does not reset seats attributes' do + expect do + gitlab_subscription.update!(seats_in_use: 99) + end.to not_change(gitlab_subscription, :max_seats_used) + .and not_change(gitlab_subscription, :seats_owed) + end + end end context 'after_destroy_commit' do diff --git a/ee/spec/requests/api/namespaces_spec.rb b/ee/spec/requests/api/namespaces_spec.rb index f8108f86e2ba6f7a0f7f043e944eef342d7a2040..9b45fe860ef64dfe8d5391afdec47b8ba24d4aac 100644 --- a/ee/spec/requests/api/namespaces_spec.rb +++ b/ee/spec/requests/api/namespaces_spec.rb @@ -590,14 +590,14 @@ def do_put(namespace_id, current_user, payload) let_it_be(:premium_plan) { create(:premium_plan) } let_it_be(:namespace) { create(:group, name: 'test.test-group.22') } - let_it_be(:gitlab_subscription) { create(:gitlab_subscription, namespace: namespace) } + let_it_be(:gitlab_subscription) { create(:gitlab_subscription, namespace: namespace, start_date: '2018/01/01', end_date: '2019/01/01') } let(:params) do { seats: 150, plan_code: 'premium', - start_date: '01/01/2018', - end_date: '01/01/2019' + start_date: '2018/01/01', + end_date: '2019/01/01' } end @@ -679,6 +679,26 @@ def do_put(namespace_id, current_user, payload) do_put(namespace.id, admin, gitlab_subscription.attributes) end.to change { gitlab_subscription.reload.updated_at } end + + context 'when starting a new term' do + it 'resets the seat attributes for the subscription' do + gitlab_subscription.update!(seats: 20, max_seats_used: 42, seats_owed: 22) + + new_start = gitlab_subscription.end_date + 1.year + new_end = gitlab_subscription.end_date + 1.year + new_term_params = params.merge(start_date: new_start, end_date: new_end) + + expect(gitlab_subscription.seats_in_use).to eq 0 + + do_put(namespace.id, admin, new_term_params) + + expect(response).to have_gitlab_http_status(:ok) + expect(gitlab_subscription.reload).to have_attributes( + max_seats_used: 0, + seats_owed: 0 + ) + end + end end end