From fb72d73babcf3b743da2ef5310569a46a97b236a Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Tue, 3 Jun 2025 10:41:51 +0200 Subject: [PATCH] [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 --- .../admin/enterprise_tokens/row_component.rb | 10 +- .../enterprise_tokens/table_component.rb | 8 +- .../users/index_page_header_component.rb | 9 +- app/models/enterprise_token.rb | 29 ++-- spec/models/enterprise_token_spec.rb | 140 +++++++++++++++--- spec/support/enterprise_token_factory.rb | 98 ++++++++++++ spec/views/users/index.html.erb_spec.rb | 4 +- 7 files changed, 246 insertions(+), 52 deletions(-) create mode 100644 spec/support/enterprise_token_factory.rb diff --git a/app/components/admin/enterprise_tokens/row_component.rb b/app/components/admin/enterprise_tokens/row_component.rb index 6577a98e926..8a30e750ff8 100644 --- a/app/components/admin/enterprise_tokens/row_component.rb +++ b/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 diff --git a/app/components/admin/enterprise_tokens/table_component.rb b/app/components/admin/enterprise_tokens/table_component.rb index c9e0c2d06ef..bd7fbebe898 100644 --- a/app/components/admin/enterprise_tokens/table_component.rb +++ b/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 diff --git a/app/components/users/index_page_header_component.rb b/app/components/users/index_page_header_component.rb index d3376e24784..243af54b74f 100644 --- a/app/components/users/index_page_header_component.rb +++ b/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 diff --git a/app/models/enterprise_token.rb b/app/models/enterprise_token.rb index 76bd8694d67..1f1fc763c5e 100644 --- a/app/models/enterprise_token.rb +++ b/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 diff --git a/spec/models/enterprise_token_spec.rb b/spec/models/enterprise_token_spec.rb index f47dbc984a0..56b6a017b63 100644 --- a/spec/models/enterprise_token_spec.rb +++ b/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 diff --git a/spec/support/enterprise_token_factory.rb b/spec/support/enterprise_token_factory.rb new file mode 100644 index 00000000000..0e2a81670dd --- /dev/null +++ b/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 diff --git a/spec/views/users/index.html.erb_spec.rb b/spec/views/users/index.html.erb_spec.rb index 9fa0b6d9ffc..15fcb5af24b 100644 --- a/spec/views/users/index.html.erb_spec.rb +++ b/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