diff --git a/app/contracts/time_entries/base_contract.rb b/app/contracts/time_entries/base_contract.rb
index eead32d5cc..a0c0429bed 100644
--- a/app/contracts/time_entries/base_contract.rb
+++ b/app/contracts/time_entries/base_contract.rb
@@ -36,6 +36,14 @@ module TimeEntries
TimeEntry
end
+ def validate
+ validate_hours_are_in_range
+ validate_project_is_set
+ validate_work_package
+
+ super
+ end
+
attribute :project_id
attribute :work_package_id
attribute :activity_id
@@ -45,5 +53,32 @@ module TimeEntries
attribute :tyear
attribute :tmonth
attribute :tweek
+
+ private
+
+ def validate_work_package
+ return unless model.work_package || model.work_package_id_changed?
+
+ if work_package_invisible? ||
+ work_package_not_in_project?
+ errors.add :work_package_id, :invalid
+ end
+ end
+
+ def validate_hours_are_in_range
+ errors.add :hours, :invalid if model.hours&.negative?
+ end
+
+ def validate_project_is_set
+ errors.add :project_id, :invalid if model.project.nil?
+ end
+
+ def work_package_invisible?
+ model.work_package.nil? || !model.work_package.visible?(user)
+ end
+
+ def work_package_not_in_project?
+ model.work_package && model.project != model.work_package.project
+ end
end
end
diff --git a/app/contracts/time_entries/update_contract.rb b/app/contracts/time_entries/update_contract.rb
new file mode 100644
index 0000000000..2bf0084d12
--- /dev/null
+++ b/app/contracts/time_entries/update_contract.rb
@@ -0,0 +1,58 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
+#
+# 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-2017 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 doc/COPYRIGHT.rdoc for more details.
+#++
+
+module TimeEntries
+ class UpdateContract < BaseContract
+ def validate
+ unless user_allowed_to_update?
+ errors.add :base, :error_unauthorized
+ end
+
+ super
+ end
+
+ private
+
+ ##
+ # Users may update time entries IFF
+ # they have the :edit_time_entries or
+ # user == editing user and :edit_own_time_entries
+ def user_allowed_to_update?
+ edit_all = user.allowed_to?(:edit_time_entries, model.project)
+ edit_own = user.allowed_to?(:edit_own_time_entries, model.project)
+
+ if model.user == user
+ edit_own || edit_all
+ else
+ edit_all
+ end
+ end
+ end
+end
diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb
index ff1028fcb2..53aef44769 100644
--- a/app/controllers/timelog_controller.rb
+++ b/app/controllers/timelog_controller.rb
@@ -163,7 +163,7 @@ class TimelogController < ApplicationController
@time_entry = call.result
- respond_for_saving call.success?
+ respond_for_saving call
end
def edit
@@ -174,8 +174,8 @@ class TimelogController < ApplicationController
def update
service = TimeEntries::UpdateService.new user: current_user, time_entry: @time_entry
- result = service.call(attributes: permitted_params.time_entry)
- respond_for_saving result.success?
+ call = service.call(attributes: permitted_params.time_entry)
+ respond_for_saving call
end
def destroy
@@ -256,13 +256,10 @@ class TimelogController < ApplicationController
time_entry
end
- def save_time_entry_and_respond(time_entry)
- call_hook(:controller_timelog_edit_before_save, params: params, time_entry: time_entry)
- respond_for_saving @time_entry.save
- end
+ def respond_for_saving(call)
+ @errors = call.errors
- def respond_for_saving(success)
- if success
+ if call.success?
respond_to do |format|
format.html do
flash[:notice] = l(:notice_successful_update)
diff --git a/app/helpers/error_message_helper.rb b/app/helpers/error_message_helper.rb
index 6d5269022a..8d521dbe70 100644
--- a/app/helpers/error_message_helper.rb
+++ b/app/helpers/error_message_helper.rb
@@ -37,11 +37,20 @@ module ErrorMessageHelper
render_error_messages_partial(error_messages, options[:object])
end
+ # Will take a contract to display the errors in a rails form.
+ # In order to have faulty field highlighted, the method sets
+ # all errors in the contract on the object as well.
def error_messages_for_contract(object, errors)
return unless errors
error_messages = errors.full_messages
+ errors.details.each do |attribute, details|
+ details.map { |d| d[:error] }.flatten.each do |message|
+ object.errors.add(attribute, message)
+ end
+ end
+
render_error_messages_partial(error_messages, object)
end
diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb
index d66e95343f..f775a2708e 100644
--- a/app/models/time_entry.rb
+++ b/app/models/time_entry.rb
@@ -49,10 +49,6 @@ class TimeEntry < ActiveRecord::Base
validates_numericality_of :hours, allow_nil: true, message: :invalid
validates_length_of :comments, maximum: 255, allow_nil: true
- validate :validate_hours_are_in_range
- validate :validate_project_is_set
- validate :validate_consistency_of_work_package_id
-
scope :on_work_packages, ->(work_packages) { where(work_package_id: work_packages) }
def self.visible(*args)
@@ -105,20 +101,4 @@ class TimeEntry < ActiveRecord::Base
activity.root
end
end
-
- private
-
- # TODO: move to contract
-
- def validate_hours_are_in_range
- errors.add :hours, :invalid if hours&.negative?
- end
-
- def validate_project_is_set
- errors.add :project_id, :invalid if project.nil?
- end
-
- def validate_consistency_of_work_package_id
- errors.add :work_package_id, :invalid if (work_package_id && !work_package) || (work_package && project != work_package.project)
- end
end
diff --git a/app/services/time_entries/update_service.rb b/app/services/time_entries/update_service.rb
index 516175531d..c2a746cb7f 100644
--- a/app/services/time_entries/update_service.rb
+++ b/app/services/time_entries/update_service.rb
@@ -29,18 +29,20 @@
#++
class TimeEntries::UpdateService
+ include Concerns::Contracted
attr_accessor :user, :time_entry
def initialize(user:, time_entry:)
self.user = user
self.time_entry = time_entry
+ self.contract_class = TimeEntries::UpdateContract
end
def call(attributes: {})
set_attributes attributes
- success = validate_and_save
- ServiceResult.new success: success, errors: time_entry.errors, result: time_entry
+ success, errors = validate_and_save(time_entry, user)
+ ServiceResult.new success: success, errors: errors, result: time_entry
end
private
@@ -50,27 +52,8 @@ class TimeEntries::UpdateService
##
# Update project context if moving time entry
- if time_entry.work_package_id_changed?
+ if time_entry.work_package && time_entry.work_package_id_changed?
time_entry.project_id = time_entry.work_package.project_id
end
end
-
- def validate_and_save
- ##
- # Perform additional validations on the model,
- # since the errors from reform are not merged into the model for form errors
- validate_visible_work_package
-
- if time_entry.errors.empty?
- time_entry.save
- else
- false
- end
- end
-
- def validate_visible_work_package
- if time_entry.work_package
- time_entry.errors.add :work_package_id, :invalid unless time_entry.work_package.visible?(user)
- end
- end
end
diff --git a/app/views/timelog/edit.html.erb b/app/views/timelog/edit.html.erb
index 65c0bfd86d..1e50d7b182 100644
--- a/app/views/timelog/edit.html.erb
+++ b/app/views/timelog/edit.html.erb
@@ -29,7 +29,7 @@ See docs/COPYRIGHT.rdoc for more details.
<%= toolbar title: t(:label_spent_time) %>
<%= labelled_tabular_form_for [@time_entry.project, @time_entry], as: :time_entry do |f| %>
- <%= error_messages_for 'time_entry' %>
+ <%= error_messages_for_contract @time_entry, @errors %>
<%= back_url_hidden_field_tag %>
diff --git a/docs/api/apiv3/endpoints/time_entries.apib b/docs/api/apiv3/endpoints/time_entries.apib
index 646040891b..878684ea67 100644
--- a/docs/api/apiv3/endpoints/time_entries.apib
+++ b/docs/api/apiv3/endpoints/time_entries.apib
@@ -331,6 +331,91 @@ There is no form and schema resource for time entries, yet.
}
}
+## Update Time entry [PATCH]
+
+Updates the given time entry by applying the attributes provided in the body. Please note that while there is a fixed set of attributes, custom fields can extend a time entries' attributes and are accepted by the endpoint.
+
+There is no form and schema resource for time entries, yet.
+
++ Request Update time entry
+
+ + Body
+
+ {
+ "_links": {
+ "activity": {
+ "href": "/api/v3/time_entries/activities/18",
+ },
+ "workPackage": {
+ "href": "/api/v3/work_packages/5"
+ },
+ "customField4": {
+ "href": "/api/v3/users/5"
+ },
+ "customField51": {
+ "href": "/api/v3/custom_options/11"
+ }
+ },
+ "hours": 'PT5H',
+ "comment": "some comment",
+ "spentOn": "2017-07-28",
+ "customField1": {
+ raw: 'some text custom field value'
+ },
+ "customField8": 5
+ }
+
++ Response 200
+
+ [Time entry][]
+
++ Response 400 (application/hal+json)
+
+ Occurs when the client did not send a valid JSON object in the request body.
+
+ + Body
+
+ {
+ "_type": "Error",
+ "errorIdentifier": "urn:openproject-org:api:v3:errors:InvalidRequestBody",
+ "message": "The request body was not a single JSON object."
+ }
+
++ Response 403 (application/hal+json)
+
+ Returned if the client does not have sufficient permissions.
+
+ **Required permission:** Edit (own) time entries, depending on what time entry is being modified.
+
+ + Body
+
+ {
+ "_type": "Error",
+ "errorIdentifier": "urn:openproject-org:api:v3:errors:MissingPermission",
+ "message": "You are not authorized to access this resource."
+ }
+
++ Response 422 (application/hal+json)
+
+ Returned if:
+
+ * a constraint for a property was violated (`PropertyConstraintViolation`)
+
+ + Body
+
+ {
+ "_type": "Error",
+ "errorIdentifier": "urn:openproject-org:api:v3:errors:PropertyConstraintViolation",
+ "message": "Work package is invalid.",
+ "_embedded": {
+ "details": {
+ "attribute"=>"workPackage"
+ }
+ }
+ }
+
+
+
# Group Time Entry Activities
diff --git a/lib/api/v3/time_entries/time_entries_api.rb b/lib/api/v3/time_entries/time_entries_api.rb
index 1ae5894782..c19296eb02 100644
--- a/lib/api/v3/time_entries/time_entries_api.rb
+++ b/lib/api/v3/time_entries/time_entries_api.rb
@@ -85,6 +85,26 @@ module API
current_user: current_user,
embed_links: true)
end
+
+ patch do
+ params = API::V3::ParseResourceParamsService
+ .new(current_user, TimeEntry, TimeEntryRepresenter)
+ .call(request_body)
+ .result
+
+ result = ::TimeEntries::UpdateService
+ .new(time_entry: @time_entry, user: current_user)
+ .call(attributes: params)
+
+ if result.success?
+ updated_entry = result.result
+ TimeEntryRepresenter.create(updated_entry,
+ current_user: current_user,
+ embed_links: true)
+ else
+ fail ::API::Errors::ErrorBase.create_and_merge_errors(result.errors)
+ end
+ end
end
mount ::API::V3::TimeEntries::TimeEntriesActivityAPI
diff --git a/spec/contracts/time_entries/create_contract_spec.rb b/spec/contracts/time_entries/create_contract_spec.rb
index 192b8455f7..74f1e6860b 100644
--- a/spec/contracts/time_entries/create_contract_spec.rb
+++ b/spec/contracts/time_entries/create_contract_spec.rb
@@ -28,147 +28,46 @@
#++
require 'spec_helper'
+require_relative './shared_contract_examples'
describe TimeEntries::CreateContract do
- let(:current_user) do
- FactoryBot.build_stubbed(:user) do |user|
- allow(user)
- .to receive(:allowed_to?) do |permission, permission_project|
- permissions.include?(permission) && time_entry_project == permission_project
- end
- end
- end
- let(:other_user) { FactoryBot.build_stubbed(:user) }
- let(:time_entry_work_package) do
- FactoryBot.build_stubbed(:work_package,
- project: time_entry_project)
- end
- let(:time_entry_project) { FactoryBot.build_stubbed(:project) }
- let(:time_entry_user) { current_user }
- let(:time_entry_activity) { FactoryBot.build_stubbed(:time_entry_activity) }
- let(:time_entry_spent_on) { Date.today }
- let(:time_entry_hours) { 5 }
- let(:time_entry_comments) { "A comment" }
- let(:time_entry) do
- TimeEntry.new(project: time_entry_project,
- work_package: time_entry_work_package,
- user: time_entry_user,
- activity: time_entry_activity,
- spent_on: time_entry_spent_on,
- hours: time_entry_hours,
- comments: time_entry_comments)
- end
- let(:permissions) { %i(log_time) }
-
- subject(:contract) { described_class.new(time_entry, current_user) }
-
- def expect_valid(valid, symbols = {})
- expect(contract.validate).to eq(valid)
-
- symbols.each do |key, arr|
- expect(contract.errors.symbols_for(key)).to match_array arr
- end
- end
-
- shared_examples 'is valid' do
- it 'is valid' do
- expect_valid(true)
- end
- end
-
- it_behaves_like 'is valid'
-
- context 'when user is not allowed to log time' do
- let(:permissions) { [] }
-
- it 'is invalid' do
- expect_valid(false, base: %i(error_unauthorized))
- end
- end
-
- context 'when time_entry user is not contract user' do
- let(:time_entry_user) { other_user }
-
- it 'is invalid' do
- expect_valid(false, user_id: %i(invalid))
- end
- end
-
- context 'when the work_package is within a differnt project than the provided project' do
- let(:time_entry_work_package) { FactoryBot.build_stubbed(:work_package) }
-
- it 'is invalid' do
- expect_valid(false, work_package_id: %i(invalid))
- end
- end
-
- context 'when the project is nil' do
- let(:time_entry_project) { nil }
-
- it 'is invalid' do
- expect_valid(false, project_id: %i(invalid blank))
- end
- end
-
- context 'when the user is nil' do
- let(:time_entry_user) { nil }
-
- it 'is invalid' do
- expect_valid(false, user_id: %i(blank invalid))
- end
- end
-
- context 'when activity is nil' do
- let(:time_entry_activity) { nil }
-
- it 'is invalid' do
- expect_valid(false, activity_id: %i(blank))
- end
- end
-
- context 'when spent_on is nil' do
- let(:time_entry_spent_on) { nil }
-
- it 'is invalid' do
- expect_valid(false, spent_on: %i(blank))
+ it_behaves_like 'time entry contract' do
+ let(:time_entry) do
+ TimeEntry.new(project: time_entry_project,
+ work_package: time_entry_work_package,
+ user: time_entry_user,
+ activity: time_entry_activity,
+ spent_on: time_entry_spent_on,
+ hours: time_entry_hours,
+ comments: time_entry_comments)
end
- end
+ let(:permissions) { %i(log_time) }
+ let(:other_user) { FactoryBot.build_stubbed(:user) }
- context 'when hours is nil' do
- let(:time_entry_hours) { nil }
+ subject(:contract) { described_class.new(time_entry, current_user) }
- it 'is invalid' do
- expect_valid(false, hours: %i(blank))
- end
- end
+ context 'when user is not allowed to log time' do
+ let(:permissions) { [] }
- context 'when hours is smaller negative' do
- let(:time_entry_hours) { -1 }
-
- it 'is invalid' do
- expect_valid(false, hours: %i(invalid))
+ it 'is invalid' do
+ expect_valid(false, base: %i(error_unauthorized))
+ end
end
- end
- context 'when hours is nil' do
- let(:time_entry_hours) { nil }
+ context 'when time_entry user is not contract user' do
+ let(:time_entry_user) { other_user }
- it 'is invalid' do
- expect_valid(false, hours: %i(blank))
+ it 'is invalid' do
+ expect_valid(false, user_id: %i(invalid))
+ end
end
- end
- context 'when comment is longer than 255' do
- let(:time_entry_comments) { "a" * 256 }
+ context 'when the user is nil' do
+ let(:time_entry_user) { nil }
- it 'is invalid' do
- expect_valid(false, comments: %i(too_long))
+ it 'is invalid' do
+ expect_valid(false, user_id: %i(blank invalid))
+ end
end
end
-
- context 'when comment is nil' do
- let(:time_entry_comments) { nil }
-
- it_behaves_like 'is valid'
- end
end
diff --git a/spec/contracts/time_entries/shared_contract_examples.rb b/spec/contracts/time_entries/shared_contract_examples.rb
new file mode 100644
index 0000000000..64765e2459
--- /dev/null
+++ b/spec/contracts/time_entries/shared_contract_examples.rb
@@ -0,0 +1,147 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# 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-2017 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 docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+shared_examples_for 'time entry contract' do
+ let(:current_user) do
+ FactoryBot.build_stubbed(:user) do |user|
+ allow(user)
+ .to receive(:allowed_to?) do |permission, permission_project|
+ permissions.include?(permission) && time_entry_project == permission_project
+ end
+ end
+ end
+ let(:other_user) { FactoryBot.build_stubbed(:user) }
+ let(:time_entry_work_package) do
+ FactoryBot.build_stubbed(:work_package,
+ project: time_entry_project)
+ end
+ let(:time_entry_project) { FactoryBot.build_stubbed(:project) }
+ let(:time_entry_user) { current_user }
+ let(:time_entry_activity) { FactoryBot.build_stubbed(:time_entry_activity) }
+ let(:time_entry_spent_on) { Date.today }
+ let(:time_entry_hours) { 5 }
+ let(:time_entry_comments) { "A comment" }
+ let(:work_package_visible) { true }
+
+ before do
+ allow(time_entry_work_package)
+ .to receive(:visible?)
+ .with(current_user)
+ .and_return(work_package_visible)
+ end
+
+ def expect_valid(valid, symbols = {})
+ expect(contract.validate).to eq(valid)
+
+ symbols.each do |key, arr|
+ expect(contract.errors.symbols_for(key)).to match_array arr
+ end
+ end
+
+ shared_examples 'is valid' do
+ it 'is valid' do
+ expect_valid(true)
+ end
+ end
+
+ it_behaves_like 'is valid'
+
+ context 'when the work_package is within a different project than the provided project' do
+ let(:time_entry_work_package) { FactoryBot.build_stubbed(:work_package) }
+
+ it 'is invalid' do
+ expect_valid(false, work_package_id: %i(invalid))
+ end
+ end
+
+ context 'when the project is nil' do
+ let(:time_entry_project) { nil }
+
+ it 'is invalid' do
+ expect_valid(false, project_id: %i(invalid blank))
+ end
+ end
+
+ context 'when activity is nil' do
+ let(:time_entry_activity) { nil }
+
+ it 'is invalid' do
+ expect_valid(false, activity_id: %i(blank))
+ end
+ end
+
+ context 'when spent_on is nil' do
+ let(:time_entry_spent_on) { nil }
+
+ it 'is invalid' do
+ expect_valid(false, spent_on: %i(blank))
+ end
+ end
+
+ context 'when hours is nil' do
+ let(:time_entry_hours) { nil }
+
+ it 'is invalid' do
+ expect_valid(false, hours: %i(blank))
+ end
+ end
+
+ context 'when hours is negative' do
+ let(:time_entry_hours) { -1 }
+
+ it 'is invalid' do
+ expect_valid(false, hours: %i(invalid))
+ end
+ end
+
+ context 'when hours is nil' do
+ let(:time_entry_hours) { nil }
+
+ it 'is invalid' do
+ expect_valid(false, hours: %i(blank))
+ end
+ end
+
+ context 'when comment is longer than 255' do
+ let(:time_entry_comments) { "a" * 256 }
+
+ it 'is invalid' do
+ expect_valid(false, comments: %i(too_long))
+ end
+ end
+
+ context 'when comment is nil' do
+ let(:time_entry_comments) { nil }
+
+ it_behaves_like 'is valid'
+ end
+end
diff --git a/spec/contracts/time_entries/update_contract_spec.rb b/spec/contracts/time_entries/update_contract_spec.rb
new file mode 100644
index 0000000000..6ed1c41f7d
--- /dev/null
+++ b/spec/contracts/time_entries/update_contract_spec.rb
@@ -0,0 +1,90 @@
+#-- encoding: UTF-8
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# 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-2017 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 docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require_relative './shared_contract_examples'
+
+describe TimeEntries::UpdateContract do
+ it_behaves_like 'time entry contract' do
+ let(:time_entry) do
+ FactoryBot.build_stubbed(:time_entry,
+ project: time_entry_project,
+ work_package: time_entry_work_package,
+ user: time_entry_user,
+ activity: time_entry_activity,
+ spent_on: time_entry_spent_on,
+ hours: time_entry_hours,
+ comments: time_entry_comments)
+ end
+ subject(:contract) { described_class.new(time_entry, current_user) }
+ let(:permissions) { %i(edit_time_entries) }
+
+ context 'when user is not allowed to edit time entries' do
+ let(:permissions) { [] }
+
+ it 'is invalid' do
+ expect_valid(false, base: %i(error_unauthorized))
+ end
+ end
+
+ context 'when the user is nil' do
+ let(:time_entry_user) { nil }
+
+ it 'is invalid' do
+ expect_valid(false, user_id: %i(blank))
+ end
+ end
+
+ context 'when the user is changed' do
+ it 'is invalid' do
+ time_entry.user = other_user
+ expect_valid(false, user_id: %i(error_readonly))
+ end
+ end
+
+ context 'when time_entry user is not contract user' do
+ let(:time_entry_user) { other_user }
+
+ context 'when has permission' do
+ let(:permissions) { %i[edit_time_entries] }
+
+ it 'is valid' do
+ expect_valid(true)
+ end
+ end
+
+ context 'when has no permission' do
+ let(:permissions) { %i[edit_own_time_entries] }
+ it 'is invalid' do
+ expect_valid(false, base: %i(error_unauthorized))
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/time_entry_spec.rb b/spec/models/time_entry_spec.rb
index 2eb4fb1305..bfd5af1266 100644
--- a/spec/models/time_entry_spec.rb
+++ b/spec/models/time_entry_spec.rb
@@ -69,14 +69,14 @@ describe TimeEntry, type: :model do
let(:child_activity_active) do
FactoryBot.create(:time_entry_activity,
- parent: activity,
- project: project1)
+ parent: activity,
+ project: project1)
end
let(:child_activity_inactive) do
FactoryBot.create(:time_entry_activity,
- parent: activity,
- project: project2,
- active: false)
+ parent: activity,
+ project: project2,
+ active: false)
end
before do
diff --git a/spec/requests/api/v3/time_entry_resource_spec.rb b/spec/requests/api/v3/time_entry_resource_spec.rb
index 1d04925bbc..188836308e 100644
--- a/spec/requests/api/v3/time_entry_resource_spec.rb
+++ b/spec/requests/api/v3/time_entry_resource_spec.rb
@@ -438,4 +438,59 @@ describe 'API v3 time_entry resource', type: :request do
end
end
end
+
+ describe 'PATCH api/v3/time_entries/:id' do
+ let(:path) { api_v3_paths.time_entry(time_entry.id) }
+ let(:permissions) { %i(edit_time_entries view_time_entries view_work_packages) }
+
+ let(:params) do
+ {
+ "hours": 'PT10H'
+ }
+ end
+
+ before do
+ time_entry
+ custom_value
+
+ patch path, params.to_json, 'CONTENT_TYPE' => 'application/json'
+ end
+
+ it 'updates the time entry' do
+ expect(subject.status).to eq(200)
+
+ time_entry.reload
+ expect(time_entry.hours).to eq 10
+ end
+
+ context 'when lacking permissions' do
+ let(:permissions) { %i(view_time_entries) }
+
+ it 'returns 403' do
+ expect(subject.status)
+ .to eql(403)
+ end
+ end
+
+ context 'when sending invalid params' do
+ let(:params) do
+ {
+ "_links": {
+ "workPackage": {
+ "href": api_v3_paths.work_package(work_package.id + 1)
+ }
+ }
+ }
+ end
+
+ it 'returns 422 and complains about work packages' do
+ expect(subject.status)
+ .to eql(422)
+
+ expect(subject.body)
+ .to be_json_eql("Work package is invalid.".to_json)
+ .at_path("message")
+ end
+ end
+ end
end