Merge pull request #10675 from opf/bug/42349-openproject-configure-reports-errors

fix setting:set rake task when setting already set by env or config file

lib/tasks/setting.rake

@@ -31,12 +31,9 @@ namespace :setting do
   task set: :environment do |_t, args|
     args.extras.each do |tuple|
       key, value = tuple.split('=')
-      setting = Setting.find_by(name: key)
-      if setting.nil?
-        Setting.create! name: key, value: value
-      else
-        setting.update! value: value
-      end
+      setting = Setting.find_by(name: key) || Setting.new(name: key)
+      setting.set_value! value, force: true
+      setting.save!
     end
   end
 

spec/tasks/setting_spec.rb

@@ -0,0 +1,101 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2022 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.
+#++
+
+require 'spec_helper'
+
+RSpec.describe Rake::Task, 'setting', :settings_reset do
+  describe 'setting:set' do
+    include_context 'rake' do
+      let(:task_name) { 'setting:set' }
+    end
+
+    it 'saves the setting in database' do
+      subject.invoke('email_delivery_method=something')
+      Setting.clear_cache
+      expect(Setting.find_by(name: 'email_delivery_method')&.value).to eq(:something)
+      expect(Setting.email_delivery_method).to eq(:something)
+    end
+
+    context 'if setting is overridden from config/configuration.yml file' do
+      before do
+        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'])
+      end
+
+      it 'saves the setting in database' do
+        expect { subject.invoke('email_delivery_method=something') }
+          .to change { Setting.find_by(name: 'email_delivery_method')&.value }
+          .from(nil)
+          .to(:something)
+      end
+
+      it 'keeps using the value from the file' do
+        expect(Setting.email_delivery_method).to eq(:initial_file_value)
+        expect { subject.invoke('email_delivery_method=something') }
+          .not_to change(Setting, :email_delivery_method)
+          .from(:initial_file_value)
+      end
+    end
+
+    context 'if setting is already set in database' do
+      before do
+        Setting.create!(name: 'email_delivery_method', value: 'initial_db_value')
+      end
+
+      it 'updates the setting' do
+        expect { subject.invoke('email_delivery_method=something') }
+          .to change { Setting.find_by(name: 'email_delivery_method')&.value }
+          .from(:initial_db_value)
+          .to(:something)
+      end
+
+      context 'if setting is overridden from config/configuration.yml file' do
+        before do
+          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'])
+        end
+
+        it 'updates the setting in database' do
+          expect { subject.invoke('email_delivery_method=something') }
+            .to change { Setting.find_by(name: 'email_delivery_method')&.value }
+            .from(:initial_db_value)
+            .to(:something)
+        end
+
+        it 'keeps using the value from the file' do
+          expect(Setting.email_delivery_method).to eq(:initial_file_value)
+          expect { subject.invoke('email_delivery_method=something') }
+            .not_to change(Setting, :email_delivery_method)
+            .from(:initial_file_value)
+        end
+      end
+    end
+  end
+end