diff --git a/modules/costs/config/locales/en.yml b/modules/costs/config/locales/en.yml index cf4b1dc3dc40..bbbc33a33279 100644 --- a/modules/costs/config/locales/en.yml +++ b/modules/costs/config/locales/en.yml @@ -186,3 +186,8 @@ en: js: text_are_you_sure: "Are you sure?" + + api_v3: + errors: + validation: + start_time_different_date: "Date part of startTime (%{start_time}) must be the same as the spentOn (%{spent_on}) date." diff --git a/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_representer.rb b/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_representer.rb index ef7efd5f5542..c5906a0e9d3e 100644 --- a/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_representer.rb +++ b/modules/costs/lib/api/v3/time_entries/schemas/time_entry_schema_representer.rb @@ -50,6 +50,15 @@ def self.represented_class schema :spent_on, type: "Date" + schema :start_time, + type: "DateTime", + required: -> { TimeEntry.must_track_start_and_end_time? }, + writable: -> { TimeEntry.can_track_start_and_end_time? } + + schema :end_time, + type: "DateTime", + writable: false + schema :hours, type: "Duration" diff --git a/modules/costs/lib/api/v3/time_entries/time_entry_representer.rb b/modules/costs/lib/api/v3/time_entries/time_entry_representer.rb index ad87f115e7a3..6e85f0a0eb77 100644 --- a/modules/costs/lib/api/v3/time_entries/time_entry_representer.rb +++ b/modules/costs/lib/api/v3/time_entries/time_entry_representer.rb @@ -88,6 +88,16 @@ class TimeEntryRepresenter < ::API::Decorators::Single datetime_formatter.format_duration_from_hours(represented.hours) if represented.hours end + date_time_property :start_time, + exec_context: :decorator, + getter: ->(*) { datetime_formatter.format_datetime(represented.start_timestamp, allow_nil: true) }, + if: ->(*) { TimeEntry.can_track_start_and_end_time? } + + date_time_property :end_time, + exec_context: :decorator, + getter: ->(*) { datetime_formatter.format_datetime(represented.end_timestamp, allow_nil: true) }, + if: ->(*) { TimeEntry.can_track_start_and_end_time? } + date_time_property :created_at date_time_property :updated_at @@ -129,6 +139,27 @@ def hours=(value) allow_nil: true) end + def start_time=(value) # rubocop:disable Metrics/AbcSize + ts = datetime_formatter.parse_datetime(value, "start_time", allow_nil: true) + + if ts.nil? + represented.start_timestamp = nil + return + end + + if represented.user.present? + ts = ts.in_time_zone(represented.user.time_zone) + end + + if ts.to_date == represented.spent_on + represented.start_time = ts.strftime("%H:%M") + else + raise API::Errors::Validation.new("start_time", + I18n.t("api_v3.errors.validation.start_time_different_date", + spent_on: represented.spent_on, start_time: ts.to_date)) + end + end + self.to_eager_load = [:work_package, :user, :activity, diff --git a/modules/costs/lib/costs/patches/permitted_params_patch.rb b/modules/costs/lib/costs/patches/permitted_params_patch.rb index af4c1bcdfcc8..c3d4d269c683 100644 --- a/modules/costs/lib/costs/patches/permitted_params_patch.rb +++ b/modules/costs/lib/costs/patches/permitted_params_patch.rb @@ -66,13 +66,17 @@ def user_rates end def time_entries + additional_fields = [] + + additional_fields << :start_time if TimeEntry.can_track_start_and_end_time? + params .require(:time_entry) .permit( + *additional_fields, :hours, :comments, :spent_on, - :start_time, :work_package_id, :activity_id, :project_id, diff --git a/modules/costs/spec/factories/time_entry_factory.rb b/modules/costs/spec/factories/time_entry_factory.rb index 325985cff303..43c1aac95b7b 100644 --- a/modules/costs/spec/factories/time_entry_factory.rb +++ b/modules/costs/spec/factories/time_entry_factory.rb @@ -38,5 +38,11 @@ after(:build) do |time_entry| time_entry.project ||= time_entry.work_package.project end + + trait :with_start_and_end_time do + time_zone { "Asia/Tokyo" } + start_time { 390 } # 6:30 AM + hours { 2.5 } + end end end diff --git a/modules/costs/spec/lib/api/v3/time_entries/schemas/time_entry_schema_representer_spec.rb b/modules/costs/spec/lib/api/v3/time_entries/schemas/time_entry_schema_representer_spec.rb index 3eee3e2c1383..c30734b03dff 100644 --- a/modules/costs/spec/lib/api/v3/time_entries/schemas/time_entry_schema_representer_spec.rb +++ b/modules/costs/spec/lib/api/v3/time_entries/schemas/time_entry_schema_representer_spec.rb @@ -108,6 +108,53 @@ end end + describe "startTime" do + let(:path) { "startTime" } + + before do + allow(TimeEntry).to receive_messages( + can_track_start_and_end_time?: can_track_times, + must_track_start_and_end_time?: must_track_times + ) + end + + context "when start- and end-time tracking is disabled" do + let(:can_track_times) { false } + let(:must_track_times) { false } + + it_behaves_like "has basic schema properties" do + let(:type) { "DateTime" } + let(:name) { TimeEntry.human_attribute_name("start_time") } + let(:required) { false } + let(:writable) { false } + end + end + + context "when start- and end-time tracking is enabled" do + let(:can_track_times) { true } + let(:must_track_times) { false } + + it_behaves_like "has basic schema properties" do + let(:type) { "DateTime" } + let(:name) { TimeEntry.human_attribute_name("start_time") } + let(:required) { false } + let(:writable) { true } + end + end + + context "when start- and end-time tracking is enforced" do + let(:can_track_times) { true } + let(:must_track_times) { true } + + it_behaves_like "has basic schema properties" do + let(:type) { "DateTime" } + let(:name) { TimeEntry.human_attribute_name("start_time") } + let(:required) { true } + let(:writable) { true } + end + end + end + describe "hours" do let(:path) { "hours" } diff --git a/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_parsing_spec.rb b/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_parsing_spec.rb index 0c7afd3b3dee..72b5c0bc57b2 100644 --- a/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_parsing_spec.rb +++ b/modules/costs/spec/lib/api/v3/time_entries/time_entry_representer_parsing_spec.rb @@ -81,6 +81,7 @@ "raw" => "some comment" }, "spentOn" => "2017-07-28", + "startTime" => "2017-07-28T12:30:00Z", text_custom_field.attribute_name(:camel_case) => { "raw" => "some text" } @@ -129,7 +130,7 @@ end describe "properties" do - context "spentOn" do + describe "spentOn" do it "updates spent_on" do time_entry = representer.from_hash(hash) expect(time_entry.spent_on) @@ -137,7 +138,62 @@ end end - context "hours" do + describe "startTime" do + context "when not tracking start and end time" do + before do + allow(TimeEntry).to receive_messages( + can_track_start_and_end_time?: false, + must_track_start_and_end_time?: false + ) + end + + it "does not set start_time" do + time_entry = representer.from_hash(hash) + expect(time_entry.start_time).to be_nil + end + end + + context "when tracking start and end time" do + before do + allow(TimeEntry).to receive_messages( + can_track_start_and_end_time?: true, + must_track_start_and_end_time?: false + ) + end + + context "when spent_on != start_time date" do + before do + hash["startTime"] = "1980-12-22T12:00:00Z" + end + + it "raises an error" do + expect do + representer.from_hash(hash) + end.to raise_error(API::Errors::Validation) + end + end + + it "sets start_time" do + user.pref[:time_zone] = "Asia/Tokyo" + + time_entry = representer.from_hash(hash) + + # timezone on the TimeEntry would be set to the user's TimeZone via the SetAttribute service, so we need to + # manually set it here + time_entry.time_zone = "Asia/Tokyo" + + # We are sending in 12:30:00 UTC as the start time, in Tokyo time (for 2017-07-28) that equals + # 21:30:00 in Japan Standard Time (JST), so the time should be set to 21:30 + + expect(time_entry.start_time).to eq((21 * 60) + 30) # 12:30 + + expect(time_entry.start_timestamp).to eq(DateTime.parse("2017-07-28T12:30:00Z")) + expect(time_entry.end_timestamp).to eq(DateTime.parse("2017-07-28T17:30:00Z")) + end + end + end + + describe "hours" do it "updates hours" do time_entry = representer.from_hash(hash) expect(time_entry.hours) @@ -159,7 +215,7 @@ end end - context "comment" do + describe "comment" do it "updates comment" do time_entry = representer.from_hash(hash) expect(time_entry.comments) @@ -167,7 +223,7 @@ end end - context "property custom field" do + describe "property custom field" do it "updates the custom value" do time_entry = representer.from_hash(hash) diff --git a/modules/costs/spec/requests/api/time_entry_resource_spec.rb b/modules/costs/spec/requests/api/time_entry_resource_spec.rb index 1117df243b15..1cb7b195a260 100644 --- a/modules/costs/spec/requests/api/time_entry_resource_spec.rb +++ b/modules/costs/spec/requests/api/time_entry_resource_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -330,6 +332,76 @@ .at_path("errorIdentifier") end end + + context "when start- & end-time tracking is enabled", + with_flag: { track_start_and_end_times_for_time_entries: true }, + with_settings: { allow_tracking_start_and_end_times: true } do + context "when start and end time were tracked" do + let!(:time_entry) do + create(:time_entry, :with_start_and_end_time, project:, work_package:, user: current_user) + end + + it "includes start and end timestamps in the API response" do + get api_v3_paths.time_entries + + expect(subject.body) + .to be_json_eql(time_entry.start_timestamp.utc.to_json) + .at_path("_embedded/elements/0/startTime") + + expect(subject.body) + .to be_json_eql(time_entry.end_timestamp.utc.to_json) + .at_path("_embedded/elements/0/endTime") + end + end + + context "when start and end time were not tracked" do + let!(:time_entry) do + create(:time_entry, project:, work_package:, user: current_user) + end + + it "returns nil as start and end timestamps in the API response" do + get api_v3_paths.time_entries + + expect(subject.body) + .to be_json_eql(nil.to_json) + .at_path("_embedded/elements/0/startTime") + + expect(subject.body) + .to be_json_eql(nil.to_json) + .at_path("_embedded/elements/0/endTime") + end + end + end + + context "when start- & end-time tracking is disabled", + with_flag: { track_start_and_end_times_for_time_entries: false }, + with_settings: { allow_tracking_start_and_end_times: false } do + context "when start and end time were tracked" do + let!(:time_entry) do + create(:time_entry, :with_start_and_end_time, project:, work_package:, user: current_user) + end + + it "does not include start and end time fields" do + get api_v3_paths.time_entries + + expect(subject.body).not_to have_json_path("_embedded/elements/0/startTime") + expect(subject.body).not_to have_json_path("_embedded/elements/0/endTime") + end + end + + context "when start and end time were not tracked" do + let!(:time_entry) do + create(:time_entry, project:, work_package:, user: current_user) + end + + it "does not include start and end time fields" do + get api_v3_paths.time_entries + + expect(subject.body).not_to have_json_path("_embedded/elements/0/startTime") + expect(subject.body).not_to have_json_path("_embedded/elements/0/endTime") + end + end + end end describe "GET /api/v3/time_entries/:id" do @@ -617,7 +689,7 @@ end it "removes the time_entry from the DB" do - expect(TimeEntry.exists?(time_entry.id)).to be_falsey + expect(TimeEntry).not_to exist(time_entry.id) end end @@ -627,7 +699,7 @@ end it "does not delete the time_entry" do - expect(TimeEntry.exists?(time_entry.id)).to be_truthy + expect(TimeEntry).to exist(time_entry.id) end end