From c785d0a681147d75fcfef9229fd2870a26e02160 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 25 Nov 2024 12:10:04 +0100 Subject: [PATCH 1/8] add time zone for time entries --- ...7_add_timezone_identifier_to_time_entry.rb | 6 ++++ modules/costs/app/models/time_entry.rb | 12 +++++++ modules/costs/spec/models/time_entry_spec.rb | 31 +++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb diff --git a/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb b/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb new file mode 100644 index 000000000000..445204122614 --- /dev/null +++ b/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb @@ -0,0 +1,6 @@ +class AddTimezoneIdentifierToTimeEntry < ActiveRecord::Migration[7.1] + def change + add_column :time_entries, :time_zone, :string + add_column :time_entry_journals, :time_zone, :string + end +end diff --git a/modules/costs/app/models/time_entry.rb b/modules/costs/app/models/time_entry.rb index 962f12750a5f..659885a7d4e4 100644 --- a/modules/costs/app/models/time_entry.rb +++ b/modules/costs/app/models/time_entry.rb @@ -121,6 +121,18 @@ def costs_visible_by?(usr) (user_id == usr.id && usr.allowed_in_project?(:view_own_hourly_rate, project)) end + def start_timestamp + return nil if start_time.blank? + + ActiveSupport::TimeZone[time_zone].local(spent_on.year, spent_on.month, spent_on.day, start_time / 60, start_time % 60) + end + + def end_timestamp + return nil if end_time.blank? + + ActiveSupport::TimeZone[time_zone].local(spent_on.year, spent_on.month, spent_on.day, end_time / 60, end_time % 60) + end + class << self def can_track_start_and_end_time?(_project: nil) OpenProject::FeatureDecisions.track_start_and_end_times_for_time_entries_active? && diff --git a/modules/costs/spec/models/time_entry_spec.rb b/modules/costs/spec/models/time_entry_spec.rb index 521288515a69..1d14b1f2ee1c 100644 --- a/modules/costs/spec/models/time_entry_spec.rb +++ b/modules/costs/spec/models/time_entry_spec.rb @@ -62,6 +62,7 @@ start_time: start_time, end_time: start_time + (hours * 60).to_i, user:, + time_zone: user.time_zone, rate: hourly_one, comments: "lorem") end @@ -493,6 +494,36 @@ def ensure_membership(project, user, permissions) end end + describe "#start_timestamp" do + it "returns nil if start_time is nil" do + time_entry.start_time = nil + expect(time_entry.start_timestamp).to be_nil + end + + it "generates a proper timestamp from the stored information" do + time_entry.start_time = 14 * 60 + time_entry.spent_on = Date.new(2024, 12, 24) + time_entry.time_zone = "America/Los_Angeles" + + expect(time_entry.start_timestamp.iso8601).to eq("2024-12-24T14:00:00-08:00") + end + end + + describe "#end_timestamp" do + it "returns nil if end_time is nil" do + time_entry.end_time = nil + expect(time_entry.end_timestamp).to be_nil + end + + it "generates a proper timestamp from the stored information" do + time_entry.end_time = 14 * 60 + time_entry.spent_on = Date.new(2024, 12, 24) + time_entry.time_zone = "America/Los_Angeles" + + expect(time_entry.end_timestamp.iso8601).to eq("2024-12-24T14:00:00-08:00") + end + end + describe ".must_track_start_and_end_time?" do context "with the feature flag enabled", with_flag: { track_start_and_end_times_for_time_entries: true } do context "with the allow setting enabled", with_settings: { allow_tracking_start_and_end_times: true } do From b34f38464cfdecfafdc9d75e375a0d03d39a845b Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 25 Nov 2024 15:14:49 +0100 Subject: [PATCH 2/8] exit early when no timezone is stored --- modules/costs/app/models/time_entry.rb | 2 ++ modules/costs/spec/models/time_entry_spec.rb | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/modules/costs/app/models/time_entry.rb b/modules/costs/app/models/time_entry.rb index 659885a7d4e4..b9e47e8d198b 100644 --- a/modules/costs/app/models/time_entry.rb +++ b/modules/costs/app/models/time_entry.rb @@ -123,12 +123,14 @@ def costs_visible_by?(usr) def start_timestamp return nil if start_time.blank? + return nil if time_zone.blank? ActiveSupport::TimeZone[time_zone].local(spent_on.year, spent_on.month, spent_on.day, start_time / 60, start_time % 60) end def end_timestamp return nil if end_time.blank? + return nil if time_zone.blank? ActiveSupport::TimeZone[time_zone].local(spent_on.year, spent_on.month, spent_on.day, end_time / 60, end_time % 60) end diff --git a/modules/costs/spec/models/time_entry_spec.rb b/modules/costs/spec/models/time_entry_spec.rb index 1d14b1f2ee1c..02afe96aa860 100644 --- a/modules/costs/spec/models/time_entry_spec.rb +++ b/modules/costs/spec/models/time_entry_spec.rb @@ -500,6 +500,11 @@ def ensure_membership(project, user, permissions) expect(time_entry.start_timestamp).to be_nil end + it "returns nil if timezone is nil" do + time_entry.time_zone = nil + expect(time_entry.start_timestamp).to be_nil + end + it "generates a proper timestamp from the stored information" do time_entry.start_time = 14 * 60 time_entry.spent_on = Date.new(2024, 12, 24) @@ -515,6 +520,11 @@ def ensure_membership(project, user, permissions) expect(time_entry.end_timestamp).to be_nil end + it "returns nil if timezone is nil" do + time_entry.time_zone = nil + expect(time_entry.end_timestamp).to be_nil + end + it "generates a proper timestamp from the stored information" do time_entry.end_time = 14 * 60 time_entry.spent_on = Date.new(2024, 12, 24) From 54b2f9a3489d30ac97fb11017a6005dbc0ea1801 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 26 Nov 2024 14:43:05 +0100 Subject: [PATCH 3/8] remove end_time, add timezone --- ...7_add_timezone_identifier_to_time_entry.rb | 11 +++- modules/costs/app/models/time_entry.rb | 16 ++---- modules/costs/spec/models/time_entry_spec.rb | 51 +++---------------- 3 files changed, 19 insertions(+), 59 deletions(-) diff --git a/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb b/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb index 445204122614..b75120ef873e 100644 --- a/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb +++ b/db/migrate/20241125104347_add_timezone_identifier_to_time_entry.rb @@ -1,6 +1,13 @@ class AddTimezoneIdentifierToTimeEntry < ActiveRecord::Migration[7.1] def change - add_column :time_entries, :time_zone, :string - add_column :time_entry_journals, :time_zone, :string + change_table :time_entries, bulk: true do |t| + t.string :time_zone, null: true + t.remove :end_time, type: :integer + end + + change_table :time_entry_journals, bulk: true do |t| + t.string :time_zone, null: true + t.remove :end_time, type: :integer + end end end diff --git a/modules/costs/app/models/time_entry.rb b/modules/costs/app/models/time_entry.rb index b9e47e8d198b..8bbeed8d1836 100644 --- a/modules/costs/app/models/time_entry.rb +++ b/modules/costs/app/models/time_entry.rb @@ -47,7 +47,7 @@ class TimeEntry < ApplicationRecord validates_presence_of :hours, if: -> { !ongoing? } validates_numericality_of :hours, allow_nil: true, message: :invalid - validates :start_time, :end_time, + validates :start_time, :hours, presence: true, if: -> { TimeEntry.must_track_start_and_end_time? } @@ -55,15 +55,6 @@ class TimeEntry < ApplicationRecord numericality: { only_integer: true, greater_than_or_equal_to: MIN_TIME, less_than_or_equal_to: MAX_TIME }, allow_blank: true - validates :end_time, - numericality: { - only_integer: true, - greater_than: ->(te) { te.start_time.to_i }, - less_than_or_equal_to: MAX_TIME - # TODO: nice error message - }, - allow_blank: true - scope :on_work_packages, ->(work_packages) { where(work_package_id: work_packages) } extend ::TimeEntries::TimeEntryScopes @@ -129,10 +120,11 @@ def start_timestamp end def end_timestamp - return nil if end_time.blank? + return nil if start_time.blank? return nil if time_zone.blank? + return nil if hours.blank? - ActiveSupport::TimeZone[time_zone].local(spent_on.year, spent_on.month, spent_on.day, end_time / 60, end_time % 60) + start_timestamp + hours.hours end class << self diff --git a/modules/costs/spec/models/time_entry_spec.rb b/modules/costs/spec/models/time_entry_spec.rb index 02afe96aa860..8f3fa58884d8 100644 --- a/modules/costs/spec/models/time_entry_spec.rb +++ b/modules/costs/spec/models/time_entry_spec.rb @@ -60,7 +60,6 @@ spent_on: date, hours:, start_time: start_time, - end_time: start_time + (hours * 60).to_i, user:, time_zone: user.time_zone, rate: hourly_one, @@ -436,58 +435,19 @@ def ensure_membership(project, user, permissions) end end - describe "end_time" do - it "allows blank values" do - time_entry.end_time = nil - expect(time_entry).to be_valid - end - - it "allows integer values between 0 and 1439" do - time_entry.end_time = 1337 - expect(time_entry).to be_valid - end - - it "does not allow values > 1439" do - time_entry.end_time = 1440 - expect(time_entry).not_to be_valid - end - - it "does not allow non integer values" do - time_entry.end_time = 1.5 - expect(time_entry).not_to be_valid - end - - it "does not allow negative values" do - time_entry.end_time = -42 - expect(time_entry).not_to be_valid - end - end - describe "start_time and end_time" do - it "does not allow end times smaller than the start time" do - time_entry.start_time = 10 * 60 - time_entry.end_time = 8 * 60 - - expect(time_entry).not_to be_valid - end - - context "when enforcing start and end times" do + context "when enforcing times" do before do allow(described_class).to receive(:must_track_start_and_end_time?).and_return(true) end it "validates that both values are present" do time_entry.start_time = nil - time_entry.end_time = nil expect(time_entry).not_to be_valid time_entry.start_time = 10 * 60 - expect(time_entry).not_to be_valid - - time_entry.end_time = 12 * 60 - expect(time_entry).to be_valid end end @@ -515,8 +475,8 @@ def ensure_membership(project, user, permissions) end describe "#end_timestamp" do - it "returns nil if end_time is nil" do - time_entry.end_time = nil + it "returns nil if start_time is nil" do + time_entry.start_time = nil expect(time_entry.end_timestamp).to be_nil end @@ -526,11 +486,12 @@ def ensure_membership(project, user, permissions) end it "generates a proper timestamp from the stored information" do - time_entry.end_time = 14 * 60 + time_entry.start_time = 8 * 60 + time_entry.hours = 2.5 time_entry.spent_on = Date.new(2024, 12, 24) time_entry.time_zone = "America/Los_Angeles" - expect(time_entry.end_timestamp.iso8601).to eq("2024-12-24T14:00:00-08:00") + expect(time_entry.end_timestamp.iso8601).to eq("2024-12-24T10:30:00-08:00") end end From 2737e62c5300ab51b62a0218aae827ae115fff1b Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 26 Nov 2024 15:05:04 +0100 Subject: [PATCH 4/8] fix rubocop --- modules/costs/app/models/time_entry.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/costs/app/models/time_entry.rb b/modules/costs/app/models/time_entry.rb index 8bbeed8d1836..7ca7f424c695 100644 --- a/modules/costs/app/models/time_entry.rb +++ b/modules/costs/app/models/time_entry.rb @@ -116,7 +116,7 @@ def start_timestamp return nil if start_time.blank? return nil if time_zone.blank? - ActiveSupport::TimeZone[time_zone].local(spent_on.year, spent_on.month, spent_on.day, start_time / 60, start_time % 60) + time_zone_object.local(spent_on.year, spent_on.month, spent_on.day, start_time / 60, start_time % 60) end def end_timestamp @@ -145,4 +145,8 @@ def must_track_start_and_end_time?(_project: nil) def cost_attribute hours end + + def time_zone_object + ActiveSupport::TimeZone[time_zone] + end end From ab4501dd0f29618385640bc17d0328348a609538 Mon Sep 17 00:00:00 2001 From: Bruno Pagno Date: Wed, 27 Nov 2024 17:44:52 +0100 Subject: [PATCH 5/8] check for truthy not for null --- .../op-autocompleter/op-autocompleter.component.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/shared/components/autocompleter/op-autocompleter/op-autocompleter.component.html b/frontend/src/app/shared/components/autocompleter/op-autocompleter/op-autocompleter.component.html index 26deec10a6e2..6c0a78b56768 100644 --- a/frontend/src/app/shared/components/autocompleter/op-autocompleter/op-autocompleter.component.html +++ b/frontend/src/app/shared/components/autocompleter/op-autocompleter/op-autocompleter.component.html @@ -134,7 +134,7 @@ - + + *ngSwitchCase="resource ==='subproject' || resource ==='version' || resource ==='status' || resource ==='default' || (!resource && !item.depth)"> {{ item.name }}