Commit 6efeca9a authored by Shreyas Agarwal's avatar Shreyas Agarwal Committed by James Lopez

Skip true up flag in License causes user limit check to be run instead

parent 111e48c4
......@@ -251,8 +251,9 @@ class License < ApplicationRecord
LICENSEE_ATTRIBUTES = %w[Name Email Company].freeze
validate :valid_license
validate :check_users_limit, if: :new_record?, unless: :validate_with_trueup?
validate :check_trueup, unless: :persisted?, if: :validate_with_trueup?
validate :check_users_limit, if: :new_record?, unless: [:validate_with_trueup?, :reconciliation_completed?]
validate :check_trueup, unless: [:persisted?, :reconciliation_completed?], if: :validate_with_trueup?
validate :check_restricted_user_count, if: :reconciliation_completed?
validate :not_expired, unless: :persisted?
before_validation :reset_license, if: :data_changed?
......@@ -444,6 +445,10 @@ class License < ApplicationRecord
restricted_attr(:subscription_id)
end
def reconciliation_completed?
restricted_attr(:reconciliation_completed)
end
def features_from_add_ons
add_ons.map { |name, count| FEATURES_FOR_ADD_ONS[name] if count.to_i > 0 }.compact
end
......@@ -498,8 +503,6 @@ class License < ApplicationRecord
end
def validate_with_trueup?
return false if restricted_attr(:skip_true_up)
[restricted_attr(:trueup_quantity),
restricted_attr(:trueup_from),
restricted_attr(:trueup_to)].all?(&:present?)
......@@ -692,9 +695,7 @@ class License < ApplicationRecord
end
if trueup_qty >= expected_trueup_qty
if restricted_user_count < daily_billable_users_count
add_limit_error(user_count: daily_billable_users_count)
end
check_restricted_user_count
else
message = ["You have applied a True-up for #{trueup_qty} #{"user".pluralize(trueup_qty)}"]
message << "but you need one for #{expected_trueup_qty} #{"user".pluralize(expected_trueup_qty)}."
......@@ -704,6 +705,12 @@ class License < ApplicationRecord
end
end
def check_restricted_user_count
return unless restricted_user_count && restricted_user_count < daily_billable_users_count
add_limit_error(user_count: daily_billable_users_count)
end
def add_limit_error(current_period: true, user_count:)
overage_count = overage(user_count)
......
......@@ -12,9 +12,7 @@ RSpec.describe License do
describe 'validations' do
describe '#valid_license' do
context 'when the license is provided' do
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
context 'when no license is provided' do
......@@ -22,9 +20,7 @@ RSpec.describe License do
license.data = nil
end
it 'is invalid' do
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
end
......@@ -36,17 +32,17 @@ RSpec.describe License do
create(:historical_data, recorded_at: date, active_user_count: active_user_count)
end
context 'when skip_true_up is true on the license' do
it 'does not add errors for invalid true up' do
set_restrictions(restricted_user_count: 10, trueup_quantity: 8, skip_true_up: true)
expect(license).to be_valid
context 'when reconciliation_completed is true on the license' do
before do
set_restrictions(restricted_user_count: 10, trueup_quantity: 8, reconciliation_completed: true)
end
it { is_expected.to be_valid }
end
context 'when skip_true_up is false on the license' do
context 'when reconciliation_completed is false on the license' do
it 'adds errors for invalid true up figures' do
set_restrictions(restricted_user_count: 10, trueup_quantity: 8, skip_true_up: false)
set_restrictions(restricted_user_count: 10, trueup_quantity: 8, reconciliation_completed: false)
expect(license).not_to be_valid
expect(license.errors.full_messages.to_sentence)
......@@ -54,7 +50,7 @@ RSpec.describe License do
end
end
context 'when skip_true_up is not present on the license' do
context 'when reconciliation_completed is not present on the license' do
it 'adds errors for invalid true up figures' do
set_restrictions(restricted_user_count: 10, trueup_quantity: 8)
......@@ -69,25 +65,23 @@ RSpec.describe License do
set_restrictions(restricted_user_count: 5, trueup_quantity: 10)
end
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
context 'but active users exceeds restricted user count' do
it 'is invalid' do
before do
create_list(:user, 6)
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
end
context 'when quantity is wrong' do
it 'is invalid' do
before do
set_restrictions(restricted_user_count: 5, trueup_quantity: 8)
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
context 'when previous user count is not present' do
......@@ -102,11 +96,11 @@ RSpec.describe License do
end
context 'with wrong true-up quantity' do
it 'is invalid' do
before do
create_list(:user, 2)
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
end
......@@ -115,8 +109,55 @@ RSpec.describe License do
set_restrictions(restricted_user_count: 5, trueup_quantity: 6, previous_user_count: 4)
end
it 'uses it to calculate the expected true-up' do
expect(license).to be_valid
it { is_expected.to be_valid }
end
end
describe '#check_restricted_user_count' do
context 'when reconciliation_completed is true' do
context 'when restricted_user_count is equal or more than active_user_count' do
before do
set_restrictions(restricted_user_count: 10, reconciliation_completed: true)
end
it { is_expected.to be_valid }
end
context 'when the restricted_user_count is less than active_user_count' do
before do
set_restrictions(restricted_user_count: 2, reconciliation_completed: true)
create_list(:user, 6)
create(:historical_data, recorded_at: described_class.current.starts_at, active_user_count: 100)
end
it 'add limit error' do
expect(license.valid?).to be_falsey
expect(license.errors.full_messages.to_sentence).to include(
'This GitLab installation currently has 6 active users, exceeding this license\'s limit of 2 by 4 users'
)
expect(license.errors.full_messages.to_sentence).not_to include(
'During the year before this license started'
)
end
end
end
context 'when reconciliation_completed is false' do
context 'when the restricted_user_count is less than active_user_count' do
before do
set_restrictions(restricted_user_count: 2, reconciliation_completed: false)
create_list(:user, 6)
create(:historical_data, recorded_at: described_class.current.starts_at, active_user_count: 100)
end
it 'add limit error' do
expect(license.valid?).to be_falsey
expect(license.errors.full_messages.to_sentence).to include(
'During the year before this license started'
)
end
end
end
end
......@@ -232,9 +273,7 @@ RSpec.describe License do
let!(:historical_data) { create(:historical_data, recorded_at: date, active_user_count: active_user_count) }
context 'when there is no active user count restriction' do
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
context 'without historical data' do
......@@ -250,9 +289,7 @@ RSpec.describe License do
end
context 'with previous_user_count and active users above of license limit' do
it 'is invalid' do
expect(license).to be_invalid
end
it { is_expected.not_to be_valid }
it 'shows the proper error message' do
license.valid?
......@@ -271,33 +308,25 @@ RSpec.describe License do
end
context 'when the license started' do
it 'is invalid' do
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
context 'after the license started' do
let(:date) { Date.current }
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
context 'in the year before the license started' do
let(:date) { described_class.current.starts_at - 6.months }
it 'is invalid' do
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
context 'earlier than a year before the license started' do
let(:date) { described_class.current.starts_at - 2.years }
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
end
......@@ -306,9 +335,7 @@ RSpec.describe License do
gl_license.restrictions = { active_user_count: active_user_count + 1 }
end
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
context 'when the active user count is met exactly' do
......@@ -324,9 +351,7 @@ RSpec.describe License do
describe '#not_expired' do
context "when the license doesn't expire" do
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
context 'when the license has expired' do
......@@ -334,9 +359,7 @@ RSpec.describe License do
gl_license.expires_at = Date.yesterday
end
it 'is invalid' do
expect(license).not_to be_valid
end
it { is_expected.not_to be_valid }
end
context 'when the license has yet to expire' do
......@@ -344,9 +367,7 @@ RSpec.describe License do
gl_license.expires_at = Date.tomorrow
end
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
end
......@@ -370,9 +391,7 @@ RSpec.describe License do
set_restrictions(restricted_user_count: 10, previous_user_count: 15)
end
it 'is valid' do
expect(license).to be_valid
end
it { is_expected.to be_valid }
end
end
end
......@@ -518,34 +537,26 @@ RSpec.describe License do
context 'when addon included' do
let(:plan) { 'premium' }
it 'returns true' do
is_expected.to eq(true)
end
it { is_expected.to eq(true) }
end
context 'when addon not included' do
let(:plan) { 'starter' }
it 'returns false' do
is_expected.to eq(false)
end
it { is_expected.to eq(false) }
end
context 'when plan is not set' do
let(:plan) { nil }
it 'returns false' do
is_expected.to eq(false)
end
it { is_expected.to eq(false) }
end
context 'when feature does not exists' do
let(:plan) { 'premium' }
let(:feature) { nil }
it 'returns false' do
is_expected.to eq(false)
end
it { is_expected.to eq(false) }
end
end
......@@ -1474,7 +1485,7 @@ RSpec.describe License do
trueup_quantity: opts[:trueup_quantity],
trueup_from: (date - 1.year).to_s,
trueup_to: date.to_s,
skip_true_up: opts[:skip_true_up]
reconciliation_completed: opts[:reconciliation_completed]
}.compact
end
......
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