From cb221d1e877260db05470f2da9615fa23ceeb129 Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Fri, 19 Jan 2024 21:08:46 +0000
Subject: [PATCH] Let CurrentSettings read from ApplicationSetting or
OrganizationSetting
---
.../organizations/organization_setting.rb | 8 +++
lib/gitlab/current_settings.rb | 10 +++-
lib/gitlab/fake_application_settings.rb | 4 ++
spec/lib/gitlab/current_settings_spec.rb | 56 ++++++++++++++++++-
.../organization_setting_spec.rb | 38 +++++++++++++
5 files changed, 111 insertions(+), 5 deletions(-)
diff --git a/app/models/organizations/organization_setting.rb b/app/models/organizations/organization_setting.rb
index 108531e670115..fb141c164d30f 100644
--- a/app/models/organizations/organization_setting.rb
+++ b/app/models/organizations/organization_setting.rb
@@ -2,6 +2,8 @@
module Organizations
class OrganizationSetting < ApplicationRecord
+ extend ::Organization::CurrentOrganization
+
belongs_to :organization
validates :settings, json_schema: { filename: "organization_settings" }
@@ -16,5 +18,11 @@ class OrganizationSetting < ApplicationRecord
end
end
end
+
+ def self.for_current_organization
+ return unless current_organization
+
+ current_organization.settings || current_organization.build_settings
+ end
end
end
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 64e0478734b61..a42b086319d9c 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -24,12 +24,18 @@ def expire_current_application_settings
Gitlab::SafeRequestStore.delete(:current_application_settings)
end
+ # rubocop:disable GitlabSecurity/PublicSend -- Method calls are forwarded to one of the setting classes
def method_missing(name, *args, **kwargs, &block)
- current_application_settings.send(name, *args, **kwargs, &block) # rubocop:disable GitlabSecurity/PublicSend
+ application_settings = current_application_settings
+
+ return application_settings.send(name, *args, **kwargs, &block) if application_settings.respond_to?(name)
+
+ Organizations::OrganizationSetting.for_current_organization.send(name, *args, **kwargs, &block)
end
+ # rubocop:enable GitlabSecurity/PublicSend
def respond_to_missing?(name, include_private = false)
- current_application_settings.respond_to?(name, include_private) || super
+ current_application_settings.respond_to?(name, include_private) || Organizations::OrganizationSetting.for_current_organization.respond_to?(name, include_private) || super
end
end
end
diff --git a/lib/gitlab/fake_application_settings.rb b/lib/gitlab/fake_application_settings.rb
index 0b9a4c161aeb1..da99d9b1b7063 100644
--- a/lib/gitlab/fake_application_settings.rb
+++ b/lib/gitlab/fake_application_settings.rb
@@ -46,6 +46,10 @@ def has_attribute?(key)
def method_missing(*)
nil
end
+
+ def respond_to_missing?(*)
+ true
+ end
end
end
diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
index 7fc50438c9574..17b2218e05c1c 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -88,6 +88,8 @@
end
describe '#current_application_settings', :use_clean_rails_memory_store_caching do
+ let_it_be(:organization_settings) { create(:organization_setting, restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) }
+
it 'allows keys to be called directly' do
described_class.update!(home_page_url: 'http://mydomain.com', signup_enabled: false)
@@ -97,10 +99,58 @@
expect(described_class.metrics_sample_interval).to be(15)
end
- it 'retrieves settings using ApplicationSettingFetcher' do
- expect(Gitlab::ApplicationSettingFetcher).to receive(:current_application_settings).and_call_original
+ context 'when key is in ApplicationSettingFetcher' do
+ it 'retrieves settings using ApplicationSettingFetcher' do
+ expect(Gitlab::ApplicationSettingFetcher).to receive(:current_application_settings).and_call_original
+
+ described_class.home_page_url
+ end
+ end
+
+ context 'when key is in OrganizationSetting' do
+ before do
+ allow(Gitlab::ApplicationSettingFetcher).to receive(:current_application_settings).and_return(nil)
+ end
+
+ context 'and the current organization is known' do
+ before do
+ allow(Organizations::OrganizationSetting).to receive(:for_current_organization).and_return(organization_settings)
+ end
+
+ it 'retrieves settings using OrganizationSetting' do
+ expect(described_class.restricted_visibility_levels).to eq(organization_settings.restricted_visibility_levels)
+ end
+ end
+
+ context 'and the current organization is unknown' do
+ before do
+ allow(Organizations::OrganizationSetting).to receive(:for_current_organization).and_return(nil)
+ end
+
+ it 'raises NoMethodError' do
+ expect { described_class.foo }.to raise_error(NoMethodError)
+ end
+ end
+ end
+
+ context 'when key is in both sources' do
+ it 'for test purposes, ensure the values are different' do
+ expect(
+ Gitlab::ApplicationSettingFetcher.current_application_settings.restricted_visibility_levels
+ ).not_to eq(organization_settings.restricted_visibility_levels)
+ end
- described_class.home_page_url
+ it 'prefers ApplicationSettingFetcher' do
+ expect(described_class.restricted_visibility_levels).to eq(
+ Gitlab::ApplicationSettingFetcher.current_application_settings.restricted_visibility_levels
+ )
+ end
+ end
+
+ context 'when key is in neither' do
+ it 'raises NoMethodError' do
+ expect { described_class.foo }.to raise_error(NoMethodError)
+ end
end
end
diff --git a/spec/models/organizations/organization_setting_spec.rb b/spec/models/organizations/organization_setting_spec.rb
index 376d0b7fe7709..dd90a4d0f58f4 100644
--- a/spec/models/organizations/organization_setting_spec.rb
+++ b/spec/models/organizations/organization_setting_spec.rb
@@ -4,6 +4,7 @@
RSpec.describe Organizations::OrganizationSetting, type: :model, feature_category: :cell do
let_it_be(:organization) { create(:organization) }
+ let_it_be(:organization_setting) { create(:organization_setting, organization: organization) }
describe 'associations' do
it { is_expected.to belong_to :organization }
@@ -54,4 +55,41 @@
end
end
end
+
+ describe '.for_current_organization' do
+ let(:dummy_class) { Class.new { extend Organization::CurrentOrganization } }
+
+ subject(:settings) { described_class.for_current_organization }
+
+ context 'when there is no current organization' do
+ it { is_expected.to be_nil }
+ end
+
+ context 'when there is a current organization' do
+ before do
+ dummy_class.current_organization = organization
+ end
+
+ after do
+ dummy_class.current_organization = nil
+ end
+
+ it 'returns current organization' do
+ expect(settings).to eq(organization_setting)
+ end
+
+ context 'when current organization does not have settings' do
+ before do
+ allow(organization).to receive(:settings).and_return(nil)
+ end
+
+ it 'returns new settings record' do
+ new_settings = settings
+
+ expect(new_settings.organization).to eq(organization)
+ expect(new_settings.new_record?).to eq(true)
+ end
+ end
+ end
+ end
end
--
GitLab