From 01a2df9e69ca284887fec641a10a9690bc8ad1db Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Tue, 7 Feb 2017 02:25:35 +0000
Subject: [PATCH 1/2] Merge branch '550-improve-trueup-validation' into
 'master'

Read true-up info from license and validate it

Closes #550

See merge request !1159
---
 app/models/license.rb                         | 80 ++++++++++++++-----
 .../550-fix-historical-max-validation         |  4 +
 lib/gitlab/usage_data.rb                      |  2 +-
 spec/lib/gitlab/usage_data_spec.rb            |  2 +-
 spec/models/license_spec.rb                   | 68 ++++++++++++++++
 5 files changed, 134 insertions(+), 22 deletions(-)
 create mode 100644 changelogs/unreleased/550-fix-historical-max-validation

diff --git a/app/models/license.rb b/app/models/license.rb
index 4df5e6647cbf6..91506780adb48 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 0000000000000..15faa547d9a3b
--- /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/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index a83397a74f623..a2c2366f067ea 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 ab906e463bd64..0375882459eae 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 d623be0de3525..48826c3aad98c 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
-- 
GitLab


From 093d5534f2670376848fa8586aa227e7b6e07eee Mon Sep 17 00:00:00 2001
From: Ruben Davila <rdavila84@gmail.com>
Date: Thu, 9 Feb 2017 17:24:51 -0500
Subject: [PATCH 2/2] Specify if downtime is required sor some migrations.

Build was failing due to this.
---
 .../20151215132013_add_pages_size_to_application_settings.rb    | 2 ++
 db/migrate/20160210105555_create_pages_domain.rb                | 2 ++
 ...0519203051_add_developers_can_merge_to_protected_branches.rb | 2 ++
 3 files changed, 6 insertions(+)

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 e7fb73190e83f..4854710e3f953 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 9af206143bd91..0e8507c7e9ad6 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 15ad8e8bcbb6b..285e798cfeadd 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
-- 
GitLab