Allow downgrade when no users were added in previous period

Currently a customer can't downgrade because validations will fail given
the historical max will be greater than the current active users. If
customer hasn't added more users in the previous period we should
compare against the current active users instead.

app/models/license.rb

@@ -2,7 +2,7 @@ class License < ActiveRecord::Base
   include ActionView::Helpers::NumberHelper
 
   validate :valid_license
-  validate :active_user_count, if: :new_record?, unless: :validate_with_trueup?
+  validate :check_users_limit, if: :new_record?, unless: :validate_with_trueup?
   validate :check_trueup, unless: :persisted?, if: :validate_with_trueup?
   validate :not_expired, unless: :persisted?
 
@@ -99,6 +99,10 @@ class License < ActiveRecord::Base
     restricted_attr(:previous_user_count)
   end
 
+  def current_active_users_count
+    @current_active_users_count ||= User.active.count
+  end
+
   def validate_with_trueup?
     [restricted_attr(:trueup_quantity),
      restricted_attr(:trueup_from),
@@ -127,37 +131,41 @@ class License < ActiveRecord::Base
     self.errors.add(:base, "The license key is invalid. Make sure it is exactly as you received it from GitLab Inc.")
   end
 
-  def historical_max(from, to)
+  def historical_max(from = nil, to = nil)
+    from ||= starts_at - 1.year
+    to   ||= starts_at
+
     HistoricalData.during(from..to).maximum(:active_user_count) || 0
   end
 
-  def active_user_count
+  def check_users_limit
     return unless restricted_user_count
 
-    historical_user_count = historical_max((starts_at - 1.year), starts_at)
-    overage               = historical_user_count - restricted_user_count
-
-    return if historical_user_count <= restricted_user_count
+    if previous_user_count && (historical_max <= previous_user_count)
+      return if restricted_user_count >= current_active_users_count
+    else
+      return if restricted_user_count >= historical_max
+    end
 
-    add_limit_error(user_count: historical_user_count, restricted_user_count: restricted_user_count, overage: overage)
+    overage = historical_max - restricted_user_count
+    add_limit_error(user_count: historical_max, restricted_user_count: restricted_user_count, overage: overage)
   end
 
   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
+    overage             = current_active_users_count - restricted_user_count
     expected_trueup_qty = if previous_user_count
                             max_historical - previous_user_count
                           else
-                            max_historical - active_user_count
+                            max_historical - current_active_users_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)
+      if restricted_user_count < current_active_users_count
+        add_limit_error(trueup: true, user_count: current_active_users_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)} "

spec/models/license_spec.rb

@@ -90,16 +90,6 @@ describe License do
       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)
@@ -170,7 +160,7 @@ describe License do
           gl_license.expires_at = Date.yesterday
         end
 
-        it "is valid" do
+        it "is invalid" do
           expect(license).not_to be_valid
         end
       end
@@ -185,6 +175,32 @@ describe License do
         end
       end
     end
+
+    describe 'downgrade' do
+      context 'when more users were added in previous period' do
+        before do
+          HistoricalData.create!(date: 6.months.ago, active_user_count: 15)
+
+          set_restrictions(restricted_user_count: 5, previous_user_count: 10)
+        end
+
+        it 'is invalid without a true-up' do
+          expect(license).not_to be_valid
+        end
+      end
+
+      context 'when no users were added in the previous period' do
+        before do
+          HistoricalData.create!(date: 6.months.ago, active_user_count: 15)
+
+          set_restrictions(restricted_user_count: 10, previous_user_count: 15)
+        end
+
+        it 'is valid' do
+          expect(license).to be_valid
+        end
+      end
+    end
   end
 
   describe "Class methods" do
@@ -328,4 +344,14 @@ describe License do
       build(:license, data: gl_license.export)
     end
   end
+
+  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
 end