Merge pull request #16567 from opf/bug/56771-meeting-timestamp-in-edi…

…t-form-not-the-same-as-in-details

Bug/56771 meeting timestamp in edit form not the same as in details

app/components/settings/time_zone_setting_component.rb

@@ -30,7 +30,7 @@
 
 module Settings
   ##
-  # A text field to enter numeric values.
+  # A select field to select a time zone from.
   class TimeZoneSettingComponent < ::ApplicationComponent
     options :form, :title
     options container_class: "-wide"

app/models/anonymous_user.rb

@@ -27,31 +27,25 @@
 #++
 
 class AnonymousUser < User
-  validate :validate_unique_anonymous_user, on: :create
-
-  # There should be only one AnonymousUser in the database
-  def validate_unique_anonymous_user
-    errors.add :base, "An anonymous user already exists." if AnonymousUser.any?
-  end
-
-  def available_custom_fields
-    []
-  end
-
-  # Overrides a few properties
-  def logged?; false end
-
-  def builtin?; true end
-
-  def admin; false end
+  include Users::FunctionUser
 
   def name(*_args); I18n.t(:label_user_anonymous) end
 
-  def mail; nil end
+  def self.first
+    anonymous_user = super
 
-  def time_zone; nil end
+    if anonymous_user.nil?
+      (anonymous_user = new.tap do |u|
+        u.lastname = "Anonymous"
+        u.login = ""
+        u.firstname = ""
+        u.mail = ""
+        u.status = User.statuses[:active]
+      end).save
 
-  def rss_key; nil end
+      raise "Unable to create the anonymous user." if anonymous_user.new_record?
+    end
 
-  def destroy; false end
+    anonymous_user
+  end
 end

app/models/deleted_user.rb

@@ -1,23 +1,9 @@
 class DeletedUser < User
-  validate :validate_unique_deleted_user, on: :create
-
-  # There should be only one DeletedUser in the database
-  def validate_unique_deleted_user
-    errors.add :base, "A DeletedUser already exists." if DeletedUser.any?
-  end
-
   def self.first
     super || create(type: to_s, status: statuses[:locked])
   end
 
-  # Overrides a few properties
-  def available_custom_fields = []
-  def logged? = false
-  def builtin? = true
-  def admin = false
+  include Users::FunctionUser
+
   def name(*_args) = I18n.t("user.deleted")
-  def mail = nil
-  def time_zone = nil
-  def rss_key = nil
-  def destroy = false
 end

app/models/exports/concerns/csv.rb

@@ -84,7 +84,7 @@ def format_csv(record, attribute)
       def csv_export_filename
         sane_filename(
           "#{Setting.app_title} #{title} \
-          #{format_time_as_date(Time.zone.now, '%Y-%m-%d')}.csv"
+          #{format_time_as_date(Time.zone.now, format: '%Y-%m-%d')}.csv"
         )
       end
     end

app/models/system_user.rb

@@ -31,31 +31,35 @@
 #
 
 class SystemUser < User
-  validate :validate_unique_system_user, on: :create
-
-  # There should be only one SystemUser in the database
-  def validate_unique_system_user
-    errors.add :base, "A SystemUser already exists." if SystemUser.any?
-  end
-
-  # Overrides a few properties
-  def logged?; false end
-
-  def builtin?; true end
+  include Users::FunctionUser
 
   def name(*_args); "System" end
 
-  def mail; nil end
+  def run_given
+    User.execute_as(self) do
+      yield self
+    end
+  end
 
-  def time_zone; nil end
+  def self.first
+    system_user = super
 
-  def rss_key; nil end
+    if system_user.nil?
+      system_user = new(
+        firstname: "",
+        lastname: "System",
+        login: "",
+        mail: "",
+        admin: true,
+        status: User.statuses[:active],
+        first_login: false
+      )
 
-  def destroy; false end
+      system_user.save
 
-  def run_given
-    User.execute_as(self) do
-      yield self
+      raise "Unable to create the system user." unless system_user.persisted?
     end
+
+    system_user
   end
 end

app/models/user.rb

@@ -399,15 +399,21 @@ def log_failed_login
   end
 
   def log_successful_login
-    update_attribute(:last_login_on, Time.now)
+    update_attribute(:last_login_on, Time.current)
   end
 
   def pref
     preference || build_preference
   end
 
   def time_zone
-    @time_zone ||= (pref.time_zone.blank? ? nil : ActiveSupport::TimeZone[pref.time_zone])
+    @time_zone ||= ActiveSupport::TimeZone[pref.time_zone] || ActiveSupport::TimeZone["Etc/UTC"]
+  end
+
+  def reload(*)
+    @time_zone = nil
+
+    super
   end
 
   def wants_comments_in_reverse_order?
@@ -538,46 +544,12 @@ def missing_authentication_method?
 
   # Returns the anonymous user.  If the anonymous user does not exist, it is created.  There can be only
   # one anonymous user per database.
-  def self.anonymous # rubocop:disable Metrics/AbcSize
-    RequestStore[:anonymous_user] ||=
-      begin
-        anonymous_user = AnonymousUser.first
-
-        if anonymous_user.nil?
-          (anonymous_user = AnonymousUser.new.tap do |u|
-            u.lastname = "Anonymous"
-            u.login = ""
-            u.firstname = ""
-            u.mail = ""
-            u.status = User.statuses[:active]
-          end).save
-
-          raise "Unable to create the anonymous user." if anonymous_user.new_record?
-        end
-        anonymous_user
-      end
+  def self.anonymous
+    RequestStore[:anonymous_user] ||= AnonymousUser.first
   end
 
   def self.system
-    system_user = SystemUser.first
-
-    if system_user.nil?
-      system_user = SystemUser.new(
-        firstname: "",
-        lastname: "System",
-        login: "",
-        mail: "",
-        admin: true,
-        status: User.statuses[:active],
-        first_login: false
-      )
-
-      system_user.save(validate: false)
-
-      raise "Unable to create the automatic migration user." unless system_user.persisted?
-    end
-
-    system_user
+    SystemUser.first
   end
 
   protected
@@ -689,6 +661,6 @@ def log_failed_login_timestamp
   end
 
   def self.default_admin_account_changed?
-    !User.active.find_by_login("admin").try(:current_password).try(:matches_plaintext?, "admin") # rubocop:disable Rails/DynamicFindBy
+    !User.active.find_by_login("admin").try(:current_password).try(:matches_plaintext?, "admin")
   end
 end

app/models/user_preference.rb

@@ -132,7 +132,11 @@ def high_contrast_theme?
   end
 
   def time_zone
-    super.presence || Setting.user_default_timezone.presence
+    super.presence || Setting.user_default_timezone.presence || "Etc/UTC"
+  end
+
+  def time_zone?
+    settings["time_zone"].present?
   end
 
   def daily_reminders

app/models/users/function_user.rb

@@ -0,0 +1,56 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 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 Users::FunctionUser
+  extend ActiveSupport::Concern
+
+  included do
+    validate :validate_unique_function_user, on: :create
+
+    # There should be only one such user in the database
+    def validate_unique_function_user
+      errors.add :base, "A #{self.class.name} already exists." if self.class.any?
+    end
+
+    def available_custom_fields = []
+
+    def logged? = false
+
+    def builtin? = true
+
+    def name(*_args); raise NotImplementedError end
+
+    def mail = nil
+
+    def time_zone; ActiveSupport::TimeZone[Setting.user_default_timezone.presence || "Etc/UTC"] end
+
+    def rss_key = nil
+
+    def destroy = false
+  end
+end

app/models/work_package/pdf_export/page.rb

@@ -127,7 +127,7 @@ def footer_page_nr
   end
 
   def footer_date
-    format_time(Time.zone.now, true)
+    format_time(Time.zone.now)
   end
 
   def total_page_nr_text

app/views/admin/settings/users_settings/show.html.erb

@@ -48,7 +48,6 @@ See COPYRIGHT and LICENSE files for more details.
 
     <%= render Settings::TimeZoneSettingComponent.new(
           "user_default_timezone",
-          container_class: "-slim",
           title: I18n.t("tooltip_user_default_timezone")
         )
     %>

app/views/users/_preferences.html.erb

@@ -30,6 +30,7 @@ See COPYRIGHT and LICENSE files for more details.
   <%= render Settings::TimeZoneSettingComponent.new(
     "time_zone",
     form: pref_fields,
+    include_blank: false,
     container_class: (defined? input_size) ? "-#{input_size}" : "-wide"
   )
   %>

lib_static/redmine/i18n.rb

@@ -115,18 +115,18 @@ def link_regex
       /(\[(.+?)\]\((.+?)\))/
     end
 
-    # Format the time to a date in the user time zone if one is set.
-    # If none is set and the time is in utc time zone (meaning it came from active record), format the date in the system timezone
-    # otherwise just use the date in the time zone attached to the time.
-    def format_time_as_date(time, format = nil)
+    # Formats the given time as a date string according to the user's time zone and
+    # optional specified format.
+    #
+    # @param time [Time] The time to format.
+    # @param format [String, nil] The strftime format to use for the date. If nil, the default
+    #   date format from `Setting.date_format` is used.
+    # @return [String, nil] The formatted date string, or nil if the time is not provided.
+    def format_time_as_date(time, format: nil)
       return nil unless time
 
       zone = User.current.time_zone
-      local_date = (if zone
-                      time.in_time_zone(zone)
-                    else
-                      time.utc? ? time.localtime : time
-                    end).to_date
+      local_date = time.in_time_zone(zone).to_date
 
       if format
         local_date.strftime(format)
@@ -135,18 +135,34 @@ def format_time_as_date(time, format = nil)
       end
     end
 
-    def format_time(time, include_date = true)
+    # Formats the given time as a time string according to the user's time zone
+    # and optional specified format.
+    #
+    # @param time [Time] The time to format.
+    # @param include_date [Boolean] Whether to include the date in the formatted
+    #   output. Defaults to true.
+    # @param format [String] The strftime format to use for the time. Defaults
+    #   to the format in `Setting.time_format`.
+    # @return [String, nil] The formatted time string, or nil if the time is not
+    #   provided.
+    def format_time(time, include_date: true, format: Setting.time_format)
       return nil unless time
 
-      time = time.to_time if time.is_a?(String)
       zone = User.current.time_zone
-      local = if zone
-                time.in_time_zone(zone)
-              else
-                (time.utc? ? time.to_time.localtime : time)
-              end
+      local = time.in_time_zone(zone)
+
       (include_date ? "#{format_date(local)} " : "") +
-        (Setting.time_format.blank? ? ::I18n.l(local, format: :time) : local.strftime(Setting.time_format))
+        (format.blank? ? ::I18n.l(local, format: :time) : local.strftime(format))
+    end
+
+    # Returns the offset to UTC (with utc prepended) currently active
+    # in the current users time zone. DST is factored in so the offset can
+    # shift over the course of the year
+    def formatted_time_zone_offset
+      # Doing User.current.time_zone and format that will not take heed of DST as it has no notion
+      # of a current time.
+      # https://github.com/rails/rails/issues/7297
+      "UTC#{User.current.time_zone.now.formatted_offset}"
     end
 
     def day_name(day)