Commit a6fa9670 authored by Vladlena Shumilo's avatar Vladlena Shumilo

Drop memoization at the LicenseHelper level

LicenseHelper uses extend self
- memoized data is stored at the module level
- is cached for the life of the process
- this is not the intended result
Move upgrade_plan_url to the EE::GitlabRoutingHelper
parent c62f76db
...@@ -42,6 +42,14 @@ module EE ...@@ -42,6 +42,14 @@ module EE
project_security_vulnerability_path(entity.project, entity, *args) project_security_vulnerability_path(entity.project, entity, *args)
end end
def upgrade_plan_path(group)
if group.present?
group_billings_path(group)
else
profile_billings_path
end
end
def self.url_helper(route_name) def self.url_helper(route_name)
define_method("#{route_name}_url") do |*args| define_method("#{route_name}_url") do |*args|
path = public_send(:"#{route_name}_path", *args) # rubocop:disable GitlabSecurity/PublicSend path = public_send(:"#{route_name}_path", *args) # rubocop:disable GitlabSecurity/PublicSend
......
...@@ -35,13 +35,11 @@ module LicenseHelper ...@@ -35,13 +35,11 @@ module LicenseHelper
end end
def current_license def current_license
return @current_license if defined?(@current_license) License.current
@current_license = License.current
end end
def current_license_title def current_license_title
@current_license_title ||= License.current ? License.current.plan.titleize : 'Core' License.current&.plan&.titleize || 'Core'
end end
def new_trial_url def new_trial_url
...@@ -52,15 +50,6 @@ module LicenseHelper ...@@ -52,15 +50,6 @@ module LicenseHelper
uri.to_s uri.to_s
end end
def upgrade_plan_url
group = @project&.group || @group
if group
group_billings_path(group)
else
profile_billings_path
end
end
def show_promotions?(selected_user = current_user) def show_promotions?(selected_user = current_user)
return false unless selected_user return false unless selected_user
......
...@@ -3,7 +3,8 @@ ...@@ -3,7 +3,8 @@
- if Gitlab::CurrentSettings.should_check_namespace_plan? - if Gitlab::CurrentSettings.should_check_namespace_plan?
- namespace = @project&.namespace || @group - namespace = @project&.namespace || @group
- if can?(current_user, :admin_namespace, namespace) - if can?(current_user, :admin_namespace, namespace)
= link_to _('Upgrade your plan'), upgrade_plan_url, class: 'btn btn-primary', target: target_blank ? '_blank' : '_self' - current_group = @project&.group || @group
= link_to _('Upgrade your plan'), upgrade_plan_path(current_group), class: 'btn btn-primary', target: target_blank ? '_blank' : '_self'
- elsif namespace.is_a?(Group) - elsif namespace.is_a?(Group)
%p= _('Contact an owner of group %{namespace_name} to upgrade the plan.') % { namespace_name: namespace.name } %p= _('Contact an owner of group %{namespace_name} to upgrade the plan.') % { namespace_name: namespace.name }
- else - else
......
...@@ -114,4 +114,24 @@ describe EE::GitlabRoutingHelper do ...@@ -114,4 +114,24 @@ describe EE::GitlabRoutingHelper do
expect(subject).to start_with 'http://localhost/users/auth/group_saml/metadata?group_path=foo&token=' expect(subject).to start_with 'http://localhost/users/auth/group_saml/metadata?group_path=foo&token='
end end
end end
describe '#upgrade_plan_path' do
subject { upgrade_plan_path(group) }
context 'when the group is present' do
let(:group) { build_stubbed(:group) }
it "returns the group billing path" do
expect(subject).to eq(group_billings_path(group))
end
end
context 'when the group is blank' do
let(:group) { nil }
it "returns the profile billing path" do
expect(subject).to eq(profile_billings_path)
end
end
end
end end
...@@ -65,8 +65,40 @@ describe LicenseHelper do ...@@ -65,8 +65,40 @@ describe LicenseHelper do
end end
describe '#guest_user_count' do describe '#guest_user_count' do
it 'returns the number of active guest users' do let!(:inactive_owner) { create(:user, :inactive, non_guest: true) }
expect(guest_user_count).to eq(User.active.count - User.active.excluding_guests.count) let!(:inactive_guest) { create(:user, :inactive) }
context 'when there are no active users' do
it 'returns 0' do
expect(guest_user_count).to eq(0)
end
end
context 'when there are active users and none is a guest user' do
let!(:owner1) { create(:user, non_guest: true) }
let!(:owner2) { create(:user, non_guest: true) }
it 'returns 0' do
expect(guest_user_count).to eq(0)
end
end
context 'when there are active users and some are guest users' do
let!(:owner) { create(:user, non_guest: true) }
let!(:guest) { create(:user) }
it 'returns the count of all active guest users' do
expect(guest_user_count).to eq(1)
end
end
context 'when there are active users and all are guest users' do
let!(:guest1) { create(:user) }
let!(:guest2) { create(:user) }
it 'returns the count of all active guest users' do
expect(guest_user_count).to eq(2)
end
end end
end end
...@@ -90,4 +122,43 @@ describe LicenseHelper do ...@@ -90,4 +122,43 @@ describe LicenseHelper do
end end
end end
end end
describe '#current_license' do
let(:license) { create(:license) }
it 'returns the current license' do
expect(license).to eq(license)
end
end
describe '#current_license_title' do
context 'when there is a current license' do
let!(:license) { create(:license, more_attrs) }
let(:more_attrs) { {} }
context 'and it has a custom plan associated to it' do
let(:more_attrs) { { plan: License::ULTIMATE_PLAN } }
it 'returns the plans title' do
expect(current_license_title).to eq('Ultimate')
end
end
context 'and it has the default plan associated to it' do
it 'returns the plans title' do
expect(current_license_title).to eq('Starter')
end
end
end
context 'when there is NOT a current license' do
before do
allow(License).to receive(:current).and_return(nil)
end
it 'returns default title' do
expect(current_license_title).to eq('Core')
end
end
end
end end
...@@ -23,6 +23,10 @@ FactoryBot.define do ...@@ -23,6 +23,10 @@ FactoryBot.define do
after(:build) { |user, _| user.block! } after(:build) { |user, _| user.block! }
end end
trait :inactive do
state_event { :deactivate }
end
trait :bot do trait :bot do
user_type { :alert_bot } user_type { :alert_bot }
end end
...@@ -83,12 +87,18 @@ FactoryBot.define do ...@@ -83,12 +87,18 @@ FactoryBot.define do
transient do transient do
developer_projects { [] } developer_projects { [] }
non_guest { false }
end end
after(:create) do |user, evaluator| after(:create) do |user, evaluator|
evaluator.developer_projects.each do |project| evaluator.developer_projects.each do |project|
project.add_developer(user) project.add_developer(user)
end end
if evaluator.non_guest
# default access_level for a group_member is owner
create(:group_member, user: user)
end
end end
factory :omniauth_user do factory :omniauth_user do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment