Read true-up info from license and validate it.

We're also considering the scenario where historical data is present but
we don't have the number of previous seats which is the case when some
customers upgrade from CE.

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,33 @@ class License < ActiveRecord::Base
   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_info).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
@@ -115,26 +126,49 @@ class License < ActiveRecord::Base
   end
 
   def active_user_count
-    restricted_user_count = 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
+    date_range            = (self.starts_at - 1.year)..self.starts_at
+    historical_user_count = HistoricalData.during(date_range).maximum(:active_user_count) || 0
+    overage               = historical_user_count - restricted_user_count
 
-    return unless active_user_count
+    return if historical_user_count <= restricted_user_count
 
-    return if active_user_count <= restricted_user_count
+    add_limit_error(user_count: historical_user_count, restricted_user_count: restricted_user_count, overage: overage)
+  end
 
-    overage = active_user_count - restricted_user_count
+  def check_trueup
+    trueup_qty, trueup_from, trueup_to = restrictions[:trueup_info].values_at(*%w(quantity from to))
+
+    active_user_count   = User.active.count
+    date_range          = Date.parse(trueup_from)..Date.parse(trueup_to)
+    max_historical      = HistoricalData.during(date_range).maximum(:active_user_count)
+    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)} but you need one for #{expected_trueup_qty} users. "
+      message << "Please contact sales at renewals@gitlab.com"
+
+      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(opts)
+    message =  opts[:trueup] ? "This GitLab installation currently has " : "During the year before this license started, this GitLab installation had "
+    message << "#{number_with_delimiter opts[:user_count]} active #{"user".pluralize(opts[:user_count])}, "
+    message << "exceeding this license's limit of #{number_with_delimiter opts[:restricted_user_count]} by "
+    message << "#{number_with_delimiter opts[:overage]} #{"user".pluralize(opts[: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 opts[:user_count]} #{"user".pluralize(opts[:user_count])} or contact sales at renewals@gitlab.com"
 
     self.errors.add(:base, message)
   end

changelogs/unreleased/550-fix-historical-max-validation

+---
+title: Read true-up info from license and validate it
+merge_request: 1159
+author:

lib/gitlab/usage_data.rb

@@ -62,7 +62,7 @@ module Gitlab
           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

spec/lib/gitlab/usage_data_spec.rb

@@ -77,7 +77,7 @@ describe Gitlab::UsageData do
       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)

spec/models/license_spec.rb

@@ -88,6 +88,72 @@ describe License do
           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_info: { quantity: opts[:trueup_quantity], from: (Date.today - 1.year).to_s, 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