diff --git a/app/models/license.rb b/app/models/license.rb index 4df5e6647cbf642c818d0057c6325dc12fd14592..91506780adb4834d2373305a7fe0ac48cffe4acf 100644 --- a/app/models/license.rb +++ b/app/models/license.rb @@ -2,7 +2,8 @@ class License < ActiveRecord::Base include ActionView::Helpers::NumberHelper validate :valid_license - validate :active_user_count, unless: :persisted? + validate :active_user_count, if: :new_record?, unless: :validate_with_trueup? + validate :check_trueup, unless: :persisted?, if: :validate_with_trueup? validate :not_expired, unless: :persisted? before_validation :reset_license, if: :data_changed? @@ -83,23 +84,35 @@ def respond_to_missing?(method_name, include_private = false) end def add_ons - return {} unless license? && restricted?(:add_ons) - - restrictions[:add_ons] + restricted_attr(:add_ons, {}) end def add_on?(code) add_ons[code].to_i > 0 end - def user_count - return unless self.license? && self.restricted?(:active_user_count) + def restricted_user_count + restricted_attr(:active_user_count) + end + + def previous_user_count + restricted_attr(:previous_user_count) + end - self.restrictions[:active_user_count] + def validate_with_trueup? + [restricted_attr(:trueup_quantity), + restricted_attr(:trueup_from), + restricted_attr(:trueup_to)].all?(&:present?) end private + def restricted_attr(name, default = nil) + return default unless license? && restricted?(name) + + restrictions[name] + end + def reset_current self.class.reset_current end @@ -114,27 +127,54 @@ def valid_license self.errors.add(:base, "The license key is invalid. Make sure it is exactly as you received it from GitLab Inc.") end - def active_user_count - restricted_user_count = user_count + def historical_max(from, to) + HistoricalData.during(from..to).maximum(:active_user_count) || 0 + end + def active_user_count return unless restricted_user_count - date_range = (self.starts_at - 1.year)..self.starts_at - active_user_count = HistoricalData.during(date_range).maximum(:active_user_count) || 0 + historical_user_count = historical_max((starts_at - 1.year), starts_at) + overage = historical_user_count - restricted_user_count - return unless active_user_count + return if historical_user_count <= restricted_user_count + + add_limit_error(user_count: historical_user_count, restricted_user_count: restricted_user_count, overage: overage) + end - return if active_user_count <= restricted_user_count + def check_trueup + trueup_qty = restrictions[:trueup_quantity] + trueup_from = Date.parse(restrictions[:trueup_from]) rescue (starts_at - 1.year) + trueup_to = Date.parse(restrictions[:trueup_to]) rescue starts_at + active_user_count = User.active.count + max_historical = historical_max(trueup_from, trueup_to) + overage = active_user_count - restricted_user_count + expected_trueup_qty = if previous_user_count + max_historical - previous_user_count + else + max_historical - active_user_count + end + + if trueup_qty >= expected_trueup_qty + if restricted_user_count < active_user_count + add_limit_error(trueup: true, user_count: active_user_count, restricted_user_count: restricted_user_count, overage: overage) + end + 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)}. " + message << "Please contact sales at renewals@gitlab.com" - overage = active_user_count - restricted_user_count + self.errors.add(:base, message) + end + end - message = "" - message << "During the year before this license started, this GitLab installation had " - message << "#{number_with_delimiter active_user_count} active #{"user".pluralize(active_user_count)}, " - message << "exceeding this license's limit of #{number_with_delimiter restricted_user_count} by " - message << "#{number_with_delimiter overage} #{"user".pluralize(overage)}. " + def add_limit_error(trueup: false, user_count:, restricted_user_count:, overage:) + message = trueup ? "This GitLab installation currently has " : "During the year before this license started, this GitLab installation had " + message << "#{number_with_delimiter(user_count)} active #{"user".pluralize(user_count)}, " + message << "exceeding this license's limit of #{number_with_delimiter(restricted_user_count)} by " + message << "#{number_with_delimiter(overage)} #{"user".pluralize(overage)}. " message << "Please upload a license for at least " - message << "#{number_with_delimiter active_user_count} #{"user".pluralize(active_user_count)}." + message << "#{number_with_delimiter(user_count)} #{"user".pluralize(user_count)} or contact sales at renewals@gitlab.com" self.errors.add(:base, message) end diff --git a/changelogs/unreleased/550-fix-historical-max-validation b/changelogs/unreleased/550-fix-historical-max-validation new file mode 100644 index 0000000000000000000000000000000000000000..15faa547d9a3b1e4f4e14f788f422e8396a5f862 --- /dev/null +++ b/changelogs/unreleased/550-fix-historical-max-validation @@ -0,0 +1,4 @@ +--- +title: Read true-up info from license and validate it +merge_request: 1159 +author: diff --git a/db/migrate/20151215132013_add_pages_size_to_application_settings.rb b/db/migrate/20151215132013_add_pages_size_to_application_settings.rb index e7fb73190e83fba73a04e4d43468f3fe21553f91..4854710e3f953bcac0dd2527b9dfb74d1c0bbdaa 100644 --- a/db/migrate/20151215132013_add_pages_size_to_application_settings.rb +++ b/db/migrate/20151215132013_add_pages_size_to_application_settings.rb @@ -1,4 +1,6 @@ class AddPagesSizeToApplicationSettings < ActiveRecord::Migration + DOWNTIME = false + def up add_column :application_settings, :max_pages_size, :integer, default: 100, null: false end diff --git a/db/migrate/20160210105555_create_pages_domain.rb b/db/migrate/20160210105555_create_pages_domain.rb index 9af206143bd91fd922b0cd836806338ebda0f69e..0e8507c7e9ad678210f991b5ab04ee1f6633ddbe 100644 --- a/db/migrate/20160210105555_create_pages_domain.rb +++ b/db/migrate/20160210105555_create_pages_domain.rb @@ -1,4 +1,6 @@ class CreatePagesDomain < ActiveRecord::Migration + DOWNTIME = false + def change create_table :pages_domains do |t| t.integer :project_id diff --git a/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb b/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb index 15ad8e8bcbb6b42184b5a807e9915344c960d53f..285e798cfeadddd27fced07927505d6960fe7825 100644 --- a/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb +++ b/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb @@ -1,6 +1,8 @@ class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers + DOWNTIME = false + disable_ddl_transaction! def change diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index a83397a74f62337a8562dbd156fcdc80be04a0fd..a2c2366f067ea8c3fe342958c67f6793cd4e755c 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -62,7 +62,7 @@ def license_usage_data usage_data[:license_md5] = Digest::MD5.hexdigest(license.data) usage_data[:historical_max_users] = ::HistoricalData.max_historical_user_count usage_data[:licensee] = license.licensee - usage_data[:license_user_count] = license.user_count + usage_data[:license_user_count] = license.restricted_user_count usage_data[:license_starts_at] = license.starts_at usage_data[:license_expires_at] = license.expires_at usage_data[:license_add_ons] = license.add_ons diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index ab906e463bd649f9b808b77df52d88d55fe9283a..0375882459eae3df737d91b471757bc4c0abfaa7 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -77,7 +77,7 @@ expect(subject[:licensee]).to eq(license.licensee) expect(subject[:active_user_count]).to eq(User.active.count) expect(subject[:licensee]).to eq(license.licensee) - expect(subject[:license_user_count]).to eq(license.user_count) + expect(subject[:license_user_count]).to eq(license.restricted_user_count) expect(subject[:license_starts_at]).to eq(license.starts_at) expect(subject[:license_expires_at]).to eq(license.expires_at) expect(subject[:license_add_ons]).to eq(license.add_ons) diff --git a/spec/models/license_spec.rb b/spec/models/license_spec.rb index d623be0de35250790e6726b61931b7357345a983..48826c3aad98cfce262586d035376776039a26d9 100644 --- a/spec/models/license_spec.rb +++ b/spec/models/license_spec.rb @@ -88,6 +88,74 @@ expect(license).to be_valid end end + + context 'with true-up info' do + def set_restrictions(opts) + gl_license.restrictions = { + active_user_count: opts[:restricted_user_count], + previous_user_count: opts[:previous_user_count], + trueup_quantity: opts[:trueup_quantity], + trueup_from: (Date.today - 1.year).to_s, + trueup_to: Date.today.to_s + } + end + + context 'when quantity is ok' do + before do + set_restrictions(restricted_user_count: 5, trueup_quantity: 10) + end + + it 'is valid' do + expect(license).to be_valid + end + + context 'but active users exceeds restricted user count' do + it 'is invalid' do + 6.times { create(:user) } + + expect(license).not_to be_valid + end + end + end + + context 'when quantity is wrong' do + it 'is invalid' do + set_restrictions(restricted_user_count: 5, trueup_quantity: 8) + + expect(license).not_to be_valid + end + end + + context 'when previous user count is not present' do + before do + set_restrictions(restricted_user_count: 5, trueup_quantity: 7) + end + + it 'uses current active user count to calculate the expected true-up' do + 3.times { create(:user) } + + expect(license).to be_valid + end + + context 'with wrong true-up quantity' do + it 'is invalid' do + 2.times { create(:user) } + + expect(license).not_to be_valid + end + end + end + + context 'when previous user count is present' do + before 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 + end + end + end end describe "Not expired" do