Revert timezone setting back to string, validate using canonical (#11607)

* Revert timezone setting back to string, validate using canonical

Add spec

* Update config/constants/settings/definitions.rb

Co-authored-by: Christophe Bliard <c.bliard@openproject.com>

Co-authored-by: Christophe Bliard <c.bliard@openproject.com>

app/models/setting.rb

@@ -356,8 +356,6 @@ class Setting < ApplicationRecord
       Date.parse value
     when :datetime
       DateTime.parse value
-    when :timezone
-      ActiveSupport::TimeZone.lookup_timezone(value) || value
     else
       value
     end

config/constants/settings/definition.rb

@@ -400,8 +400,6 @@ module Settings
         AR_BOOLEAN_TYPE.cast(value)
       when :symbol
         value.to_sym
-      when :timezone
-        ActiveSupport::TimeZone.lookup_timezone(value) || value
       else
         value
       end

config/constants/settings/definitions.rb

@@ -922,8 +922,8 @@ Settings::Definition.define do
 
   add :user_default_timezone,
       default: nil,
-      format: :timezone,
-      allowed: ActiveSupport::TimeZone.all + [nil]
+      format: :string,
+      allowed: ActiveSupport::TimeZone.all.map { |tz| tz.tzinfo.canonical_identifier }.sort.uniq + [nil]
 
   add :users_deletable_by_admins,
       default: false

lib/core_extensions.rb

@@ -28,7 +28,6 @@
 
 require 'core_extensions/string'
 require 'core_extensions/time_with_zone'
-require 'core_extensions/time_zone'
 
 ::String.prepend CoreExtensions::String
 ::ActiveSupport::TimeWithZone.include CoreExtensions::TimeWithZone

lib/core_extensions/time_zone.rb

@@ -1,37 +0,0 @@
-#-- 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.
-#++
-
-module CoreExtensions
-  module TimeZone # :nodoc:
-    def lookup_timezone(value)
-      all.detect { |tz| tz.name == value || tz.to_s == value || tz.tzinfo.canonical_identifier == value }
-    end
-  end
-end
-
-::ActiveSupport::TimeZone.extend ::CoreExtensions::TimeZone

spec/constants/settings/definition_spec.rb

@@ -166,9 +166,13 @@ describe Settings::Definition do
       it 'overriding timezone configuration from ENV will cast the value' do
         stub_const('ENV', { 'OPENPROJECT_USER__DEFAULT__TIMEZONE' => 'Europe/Berlin' })
 
-        value = all.detect { |d| d.name == 'user_default_timezone' }.value
-        expect(value).to be_a ::ActiveSupport::TimeZone
-        expect(value.name).to eq 'Berlin'
+        expect(value_for('user_default_timezone')).to eq 'Europe/Berlin'
+      end
+
+      it 'overriding timezone configuration from ENV with a bogus value' do
+        stub_const('ENV', { 'OPENPROJECT_USER__DEFAULT__TIMEZONE' => 'foobar' })
+
+        expect { value_for('user_default_timezone') }.to raise_error(ArgumentError)
       end
 
       it 'overriding configuration from ENV will set it to non writable' do
@@ -983,30 +987,6 @@ describe Settings::Definition do
           .to be false
       end
     end
-
-    context 'with a timezone' do
-      let(:instance) do
-        described_class.new 'timezone',
-                            format: :timezone,
-                            default: nil,
-                            allowed: ActiveSupport::TimeZone.all + [nil]
-      end
-
-      it 'accepts a TZ name' do
-        instance.value = 'Europe/Berlin'
-        expect(instance).to be_valid
-      end
-
-      it 'accepts a TZ full identifier' do
-        instance.value = '(GMT+01:00) Berlin'
-        expect(instance).to be_valid
-      end
-
-      it 'rejects a bogus value' do
-        instance.value = 'foobar'
-        expect(instance).not_to be_valid
-      end
-    end
   end
 
   describe '#on_change' do

spec/features/users/my_spec.rb

@@ -111,6 +111,22 @@ describe 'my',
       end
     end
 
+    describe 'settings' do
+      context 'with a default time zone', with_settings: { user_default_timezone: 'Asia/Tokyo' } do
+        it 'can override a time zone' do
+          expect(user.pref.time_zone).to eq 'Asia/Tokyo'
+          visit my_settings_path
+
+          expect(page).to have_select 'pref_time_zone', selected: '(GMT+09:00) Tokyo'
+          select '(GMT+01:00) Paris', from: 'pref_time_zone'
+          click_on 'Save'
+
+          expect(page).to have_select 'pref_time_zone', selected: '(GMT+01:00) Paris'
+          expect(user.pref.time_zone).to eq 'Europe/Paris'
+        end
+      end
+    end
+
     it 'in Access Tokens they can generate their API key' do
       visit my_access_token_path
       expect(page).to have_content 'Missing API access key'