[64137] Compute max allowed active users from enterprise tokens

- invalid or expired tokens are ignored, only active tokens are
  considered
- if there are active non-trial tokens, the trial tokens are ignored
- if there is an active token without any user restrictions (no
  `active_user_count` key), there is no limit on the number of active
  users
- the maximum allowed active users is computed by taking the maximum of
  the `active_user_count` values of the active tokens
- if there are no tokens, there is no limit on the number of active
  users

app/components/admin/enterprise_tokens/row_component.rb

@@ -33,10 +33,6 @@ module Admin::EnterpriseTokens
     alias :token :model
     delegate :subscriber, :domain, to: :token
 
-    def token
-      model
-    end
-
     def email
       token.mail
     end
@@ -45,11 +41,11 @@ module Admin::EnterpriseTokens
       helpers.enterprise_token_plan_name(token)
     end
 
-    def active_users
-      if token.restrictions.nil? || token.restrictions[:active_user_count].blank?
+    def max_active_users
+      if token.unlimited_users?
         I18n.t("js.admin.enterprise.upsell.unlimited")
       else
-        token.restrictions[:active_user_count]
+        token.max_active_users
       end
     end
 

app/components/admin/enterprise_tokens/table_component.rb

@@ -30,9 +30,9 @@
 
 module Admin::EnterpriseTokens
   class TableComponent < ::OpPrimer::BorderBoxTableComponent
-    columns :plan, :subscriber, :active_users, :email, :domain, :dates
+    columns :plan, :subscriber, :max_active_users, :email, :domain, :dates
 
-    mobile_columns :plan, :subscriber, :active_users, :dates
+    mobile_columns :plan, :subscriber, :max_active_users, :dates
 
     mobile_labels :project_name
 
@@ -58,10 +58,10 @@ module Admin::EnterpriseTokens
       @headers ||= [
         [:plan, { caption: EnterpriseToken.human_attribute_name(:plan) }],
         [:subscriber, { caption: EnterpriseToken.human_attribute_name(:subscriber) }],
-        [:active_users, { caption: EnterpriseToken.human_attribute_name(:active_user_count_restriction) }],
+        [:max_active_users, { caption: EnterpriseToken.human_attribute_name(:active_user_count_restriction) }],
         [:email, { caption: EnterpriseToken.human_attribute_name(:email) }],
         [:domain, { caption: EnterpriseToken.human_attribute_name(:domain) }],
-        [:dates, { caption: I18n.t(:label_dates) }],
+        [:dates, { caption: I18n.t(:label_dates) }]
       ].compact
     end
   end

app/components/users/index_page_header_component.rb

@@ -32,16 +32,11 @@ class Users::IndexPageHeaderComponent < ApplicationComponent
   include OpPrimer::ComponentHelpers
   include ApplicationHelper
 
+  delegate :user_limit, to: :"OpenProject::Enterprise"
+
   def breadcrumb_items
     [{ href: admin_index_path, text: t("label_administration") },
      { href: admin_settings_users_path, text: t(:label_user_and_permission) },
      t(:label_user_plural)]
   end
-
-  def user_limit
-    token = OpenProject::Enterprise.token
-    limit = token && Hash(token.restrictions)[:active_user_count]
-
-    limit if limit && limit > 0
-  end
 end

app/models/enterprise_token.rb

@@ -80,15 +80,11 @@ class EnterpriseToken < ApplicationRecord
     end
 
     def user_limit
-      non_trial_user_limit.presence || trial_user_limit
-    end
-
-    def non_trial_user_limit
-      active_non_trial_tokens.map { |token| Hash(token.restrictions)[:active_user_count] }.max
-    end
-
-    def trial_user_limit
-      active_trial_tokens.map { |token| Hash(token.restrictions)[:active_user_count] }.max
+      if active_non_trial_tokens.any?
+        get_user_limit_of(active_non_trial_tokens)
+      else
+        get_user_limit_of(active_trial_tokens)
+      end
     end
 
     def banner_type_for(feature:)
@@ -109,6 +105,13 @@ class EnterpriseToken < ApplicationRecord
     def clear_current_tokens_cache
       RequestStore.delete :current_ee_tokens
     end
+
+    def get_user_limit_of(tokens)
+      tokens.partition(&:unlimited_users?)
+        .find(proc { [] }, &:present?)
+        .map(&:max_active_users)
+        .max
+    end
   end
 
   FAR_FUTURE_DATE = Date.new(9999, 1, 1)
@@ -162,6 +165,14 @@ class EnterpriseToken < ApplicationRecord
     !token_object.valid_domain?(Setting.host_name)
   end
 
+  def unlimited_users?
+    max_active_users.nil?
+  end
+
+  def max_active_users
+    Hash(restrictions)[:active_user_count]
+  end
+
   def sort_key
     [expires_at || FAR_FUTURE_DATE, starts_at || FAR_FUTURE_DATE]
   end

spec/models/enterprise_token_spec.rb

@@ -31,30 +31,7 @@
 require "spec_helper"
 
 RSpec.describe EnterpriseToken do
-  before do
-    described_class.clear_current_tokens_cache
-
-    # Calls are mocked in mock_token_object for enterprise tokens created by
-    # tests. This line is to call normal implementation when not mocked.
-    allow(OpenProject::Token).to receive(:import).and_call_original
-  end
-
-  def create_enterprise_token(encoded_token_name, **attributes)
-    mock_token_object(encoded_token_name, **attributes)
-    enterprise_token = described_class.new(encoded_token: encoded_token_name)
-    enterprise_token.save!(validate: false)
-    enterprise_token
-  end
-
-  def mock_token_object(encoded_token_name, **attributes)
-    token = OpenProject::Token.new(domain: Setting.host_name,
-                                   expires_at: 1.year.from_now,
-                                   **attributes)
-    allow(OpenProject::Token)
-      .to receive(:import).with(encoded_token_name)
-                          .and_return(token)
-    token
-  end
+  include EnterpriseTokenFactory
 
   describe ".active?" do
     context "without any tokens" do
@@ -114,6 +91,69 @@ RSpec.describe EnterpriseToken do
     end
   end
 
+  describe ".user_limit" do
+    context "without any tokens" do
+      it "returns `nil` (unlimited)" do
+        expect(described_class.user_limit).to be_nil
+      end
+    end
+
+    context "when only trial tokens exist" do
+      before do
+        create_enterprise_token("trial_token_10_users", trial: true,
+                                                        restrictions: { active_user_count: 10 })
+        create_enterprise_token("trial_token_20_users", trial: true,
+                                                        restrictions: { active_user_count: 20 })
+        create_enterprise_token("trial_token_30_users_expired", trial: true,
+                                                                restrictions: { active_user_count: 30 },
+                                                                expires_at: Date.yesterday)
+        create_enterprise_token("trial_token_40_users_invalid", trial: true,
+                                                                restrictions: { active_user_count: 40 },
+                                                                domain: "invalid.domain")
+      end
+
+      it "returns the maximum seats value of active trial tokens" do
+        expect(described_class.user_limit).to eq(20)
+      end
+
+      it "returns `nil` (unlimited) if an active trial token has no seats limit" do
+        create_enterprise_token("trial_token_unlimited_users", trial: true)
+        expect(described_class.user_limit).to be_nil
+      end
+    end
+
+    context "when trial and non-trial tokens exist" do
+      before do
+        create_enterprise_token("non_trial_token_10_users", restrictions: { active_user_count: 10 })
+        create_enterprise_token("trial_token_50_users", trial: true,
+                                                        restrictions: { active_user_count: 50 })
+        create_enterprise_token("non_trial_token_20_users", restrictions: { active_user_count: 20 })
+      end
+
+      it "ignores trial tokens and returns the maximum seats value of active non-trial tokens" do
+        expect(described_class.user_limit).to eq(20)
+      end
+
+      it "returns `nil` (unlimited) if an active non-trial token has no seats limit" do
+        # unlimited token not taken into account: trial
+        create_enterprise_token("trial_token_unlimited_users", trial: true)
+        expect(described_class.user_limit).to eq(20)
+
+        # unlimited token not taken into account: invalid domain
+        create_enterprise_token("non_trial_token_unlimited_users_invalid", domain: "invalid.domain")
+        expect(described_class.user_limit).to eq(20)
+
+        # unlimited token not taken into account: expired
+        create_enterprise_token("non_trial_token_unlimited_users_expired", expires_at: Date.yesterday)
+        expect(described_class.user_limit).to eq(20)
+
+        # valid unlimited token
+        create_enterprise_token("non_trial_token_unlimited_users")
+        expect(described_class.user_limit).to be_nil
+      end
+    end
+  end
+
   describe ".banner_type_for" do
     before do
       allow(described_class).to receive(:allows_to?).with(:active_feature).and_return(true)
@@ -411,4 +451,56 @@ RSpec.describe EnterpriseToken do
       expect(described_class).not_to be_hide_banners
     end
   end
+
+  describe "#max_active_users" do
+    context "when token restrictions is nil" do
+      let(:token) { build_enterprise_token(restrictions: nil) }
+
+      it "returns nil" do
+        expect(token.max_active_users).to be_nil
+      end
+    end
+
+    context "when token restrictions does not have an active_user_count key" do
+      let(:token) { build_enterprise_token(restrictions: { foo: :bar }) }
+
+      it "returns nil" do
+        expect(token.max_active_users).to be_nil
+      end
+    end
+
+    context "when token restrictions has an active_user_count key" do
+      let(:token) { build_enterprise_token(restrictions: { active_user_count: 10 }) }
+
+      it "returns the active_user_count value" do
+        expect(token.max_active_users).to eq(10)
+      end
+    end
+  end
+
+  describe "#unlimited_users?" do
+    context "when token restrictions is nil" do
+      let(:token) { build_enterprise_token(restrictions: nil) }
+
+      it "is true" do
+        expect(token.unlimited_users?).to be true
+      end
+    end
+
+    context "when token restrictions does not have an active_user_count key" do
+      let(:token) { build_enterprise_token(restrictions: { foo: :bar }) }
+
+      it "is true" do
+        expect(token.unlimited_users?).to be true
+      end
+    end
+
+    context "when token restrictions has an active_user_count key" do
+      let(:token) { build_enterprise_token(restrictions: { active_user_count: 10 }) }
+
+      it "is false" do
+        expect(token.unlimited_users?).to be false
+      end
+    end
+  end
 end

spec/support/enterprise_token_factory.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module EnterpriseTokenFactory
+  def self.included(base)
+    base.before do
+      EnterpriseToken.clear_current_tokens_cache
+
+      # Calls are mocked in mock_token_object for enterprise tokens created by
+      # tests. This line is to call normal implementation when not mocked.
+      allow(OpenProject::Token).to receive(:import).and_call_original
+    end
+  end
+
+  # Creates a new EnterpriseToken and saves it to the database.
+  #
+  # When calling `#token_object`, it returns a real `OpenProject::Token` object
+  # built with the given attributes. `OpenProject::Token.import` is mocked to
+  # return it when called with the encoded token name.
+  #
+  # A block can be given to perform additional actions on the created
+  # EnterpriseToken, like with FactoryBot.
+  #
+  # @param encoded_token_name [String, nil] The encoded token name (optional);
+  #   use a descriptive name to identify it more easily when debugging failing
+  #   tests
+  # @param attributes [Hash] The attributes for the inner `OpenProject::Token` object
+  # @yield [EnterpriseToken] The `EnterpriseToken` instance
+  # @return [EnterpriseToken] The created `EnterpriseToken`
+  def create_enterprise_token(encoded_token_name = nil, **attributes)
+    encoded_token_name ||= "token"
+    enterprise_token = build_enterprise_token(encoded_token_name, **attributes) do |token|
+      token.save!(validate: false)
+    end
+    yield enterprise_token if block_given?
+    enterprise_token
+  end
+
+  # Builds a new EnterpriseToken without saving it to the database.
+  #
+  # When calling `#token_object`, it returns a real `OpenProject::Token` object
+  # built with the given attributes. `OpenProject::Token.import` is mocked to
+  # return it when called with the encoded token name.
+  #
+  # A block can be given to perform additional actions on the built
+  # EnterpriseToken, like with FactoryBot.
+  #
+  # @param encoded_token_name [String, nil] The encoded token name (optional);
+  #   use a descriptive name to identify it more easily when debugging failing
+  #   tests
+  # @param attributes [Hash] The attributes for the inner `OpenProject::Token` object
+  # @yield [EnterpriseToken] The `EnterpriseToken` instance
+  # @return [EnterpriseToken] The built `EnterpriseToken`
+  def build_enterprise_token(encoded_token_name = nil, **attributes)
+    encoded_token_name ||= "token"
+    mock_token_object(encoded_token_name, **attributes)
+    enterprise_token = EnterpriseToken.new(encoded_token: encoded_token_name)
+    yield enterprise_token if block_given?
+    enterprise_token
+  end
+
+  def mock_token_object(encoded_token_name, **attributes)
+    token = OpenProject::Token.new(domain: Setting.host_name,
+                                   expires_at: 1.year.from_now,
+                                   **attributes)
+    allow(OpenProject::Token)
+      .to receive(:import).with(encoded_token_name)
+                          .and_return(token)
+    token
+  end
+end

spec/views/users/index.html.erb_spec.rb

@@ -31,6 +31,8 @@
 require "spec_helper"
 
 RSpec.describe "users/index" do
+  include EnterpriseTokenFactory
+
   shared_let(:admin) { create(:admin) }
   let!(:user) { create(:user, firstname: "Scarlet", lastname: "Scallywag") }
 
@@ -57,7 +59,7 @@ RSpec.describe "users/index" do
 
   context "with an Enterprise token" do
     before do
-      allow(OpenProject::Enterprise).to receive(:token).and_return(Struct.new(:restrictions).new({ active_user_count: 5 }))
+      create_enterprise_token("token_5_users", restrictions: { active_user_count: 5 })
     end
 
     it "shows the current number of active and allowed users" do