From 51ec0870cbf2685637549bdc118e6740f4e00bc2 Mon Sep 17 00:00:00 2001
From: Christophe Bliard <c.bliard@openproject.com>
Date: Thu, 23 Jun 2022 10:35:59 +0200
Subject: [PATCH] Allow partial overriding of settings from file in test env

When Rails environment is test, overriding definition values from file
is still not possible from default section, and becomes possible from
the test section.

For instance with this `config/configuration.yml` file:

    default:
      edition: bim
    test:
      log_level: debug

In tests, log level will be debug but the edition will NOT be bim.
---
 config/constants/settings/definition.rb    | 27 +++++++++++-----------
 spec/constants/settings/definition_spec.rb | 20 +++++++++++++++-
 spec/tasks/setting_spec.rb                 |  4 ++++
 3 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb
index 0935f598607..7775830faec 100644
--- a/config/constants/settings/definition.rb
+++ b/config/constants/settings/definition.rb
@@ -141,19 +141,19 @@ module Settings
         @by_name = nil
 
         definition = new(name,
-                         format: format,
-                         default: default,
-                         writable: writable,
-                         allowed: allowed,
-                         env_alias: env_alias)
+                         format:,
+                         default:,
+                         writable:,
+                         allowed:,
+                         env_alias:)
 
         override_value(definition)
 
         all << definition
       end
 
-      def define(&block)
-        instance_exec(&block)
+      def define(&)
+        instance_exec(&)
       end
 
       def [](name)
@@ -215,18 +215,17 @@ module Settings
 
       # Replace values for which an entry in the config file or as an environment variable exists.
       def override_value(definition)
-        # The test setup should govern the configuration
-        override_value_from_file(definition) unless Rails.env.test?
+        override_value_from_file(definition)
         override_value_from_env(definition)
       end
 
       def override_value_from_file(definition)
-        name = definition.name
+        envs = ['default', Rails.env]
+        envs.delete('default') if Rails.env.test? # The test setup should govern the configuration
+        envs.each do |env|
+          next unless file_config.dig(env, definition.name)
 
-        ['default', Rails.env].each do |env|
-          next unless file_config.dig(env, name)
-
-          definition.override_value(file_config.dig(env, name))
+          definition.override_value(file_config.dig(env, definition.name))
         end
       end
 
diff --git a/spec/constants/settings/definition_spec.rb b/spec/constants/settings/definition_spec.rb
index 992420068e4..69751691b2c 100644
--- a/spec/constants/settings/definition_spec.rb
+++ b/spec/constants/settings/definition_spec.rb
@@ -402,7 +402,7 @@ describe Settings::Definition do
                 .with(Rails.root.join('config/configuration.yml'))
                 .and_return(file_contents)
 
-        # Loading of the config file is disabled in test env normally.
+        # Loading of the config file is partially disabled in test env
         allow(Rails.env)
           .to receive(:test?)
           .and_return(false)
@@ -448,6 +448,24 @@ describe Settings::Definition do
           .to eql DateTime.parse("2222-01-01")
       end
 
+      context 'when Rails environment is test' do
+        before do
+          allow(Rails.env)
+            .to receive(:test?)
+            .and_return(true)
+        end
+
+        it 'does not override from file default' do
+          expect(all.detect { |d| d.name == 'edition' }.value)
+            .not_to eql 'bim'
+        end
+
+        it 'overrides from file current env' do
+          expect(all.detect { |d| d.name == 'smtp_address' }.value)
+            .to eql 'test address'
+        end
+      end
+
       context 'when having invalid values in the file' do
         let(:file_contents) do
           <<~YAML
diff --git a/spec/tasks/setting_spec.rb b/spec/tasks/setting_spec.rb
index 527d330e7b0..3b1b3688653 100644
--- a/spec/tasks/setting_spec.rb
+++ b/spec/tasks/setting_spec.rb
@@ -43,6 +43,8 @@ RSpec.describe Rake::Task, 'setting', :settings_reset do
 
     context 'if setting is overridden from config/configuration.yml file' do
       before do
+        # disable test env detection because loading of the config file is partially disabled in test env
+        allow(Rails.env).to receive(:test?).and_return(false)
         allow(Settings::Definition).to receive(:file_config)
           .and_return('default' => { 'email_delivery_method' => 'initial_file_value' })
         Settings::Definition.send(:override_value_from_file, Settings::Definition['email_delivery_method'])
@@ -77,6 +79,8 @@ RSpec.describe Rake::Task, 'setting', :settings_reset do
 
       context 'if setting is overridden from config/configuration.yml file' do
         before do
+          # disable test env detection because loading of the config file is partially disabled in test env
+          allow(Rails.env).to receive(:test?).and_return(false)
           allow(Settings::Definition).to receive(:file_config)
             .and_return('default' => { 'email_delivery_method' => 'initial_file_value' })
           Settings::Definition.send(:override_value_from_file, Settings::Definition['email_delivery_method'])