Merge pull request #10784 from opf/fix/40921-avoid_validating_automatically_updated_wps

Fix/40921 avoid validating automatically updated wps
pull/10804/head
ulferts 2 years ago committed by GitHub
commit aee3d16dcc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 36
      app/contracts/work_packages/update_dependent_contract.rb
  2. 6
      app/services/attachments/replace_attachments.rb
  3. 6
      app/services/base_services/base_contracted.rb
  4. 2
      app/services/base_services/copy.rb
  5. 4
      app/services/base_services/create.rb
  6. 3
      app/services/journals/create_service.rb
  7. 2
      app/services/relations/base_service.rb
  8. 1
      app/services/work_packages/schedule_dependency.rb
  9. 13
      app/services/work_packages/set_attributes_service.rb
  10. 4
      app/services/work_packages/set_schedule_service.rb
  11. 167
      app/services/work_packages/update_service.rb
  12. 2
      lib_static/plugins/acts_as_customizable/lib/acts_as_customizable.rb
  13. 6
      modules/backlogs/lib/open_project/backlogs/list.rb
  14. 36
      modules/backlogs/lib/open_project/backlogs/patches/update_service_patch.rb
  15. 198
      spec/requests/api/v3/work_packages/dependent_errors_spec.rb
  16. 27
      spec/services/work_packages/set_attributes_service_spec.rb
  17. 380
      spec/services/work_packages/update_service_integration_spec.rb
  18. 235
      spec/services/work_packages/update_service_spec.rb

@ -0,0 +1,36 @@
# OpenProject is an open source project management software.
# Copyright (C) 2010-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.
# Disables all validations but still allows to be queried for assignable_* (e.g. statuses)
# that are necessary whenever a dependent (e.g. descendant of a work package) is moved to a project
# where the type is switched.
module WorkPackages
class UpdateDependentContract < UpdateContract
def validate
true
end
end
end

@ -34,13 +34,11 @@ module Attachments
private
def set_attributes(attributes)
call = super
super.tap do |call|
if call.success? && call.result.attachments_replacements
call.result.attachments = call.result.attachments_replacements
end
call
end
end
end
end

@ -50,12 +50,12 @@ module BaseServices
# Determine the type of context
# this service is running in
# e.g., within a resource lock or just executing as the given user
def service_context(&)
in_context(model, true, &)
def service_context(send_notifications: true, &block)
in_context(model, send_notifications, &block)
end
def perform(params = {})
service_context do
service_context(send_notifications: (params || {}).fetch(:send_notifications, true)) do
service_call = validate_params(params)
service_call = before_perform(params, service_call) if service_call.success?
service_call = validate_contract(service_call) if service_call.success?

@ -94,7 +94,7 @@ module BaseServices
##
# Disabling sending regular notifications
def service_context(&)
def service_context(*_args, &)
in_context(model, false, &)
end

@ -30,8 +30,8 @@ module BaseServices
class Create < Write
protected
def service_context(&)
in_user_context(true, &)
def service_context(send_notifications: true, &block)
in_user_context(send_notifications, &block)
end
def instance(_params)

@ -54,9 +54,6 @@ module Journals
return ServiceResult.new success: true unless journal
destroy_predecessor(journal)
journal
reload_journals
touch_journable(journal)

@ -70,7 +70,7 @@ class Relations::BaseService < ::BaseServices::BaseCallable
# The to-work_package will not be altered by the schedule service so
# we do not have to save the result of the service.
save_result = if schedule_result.success?
schedule_result.dependent_results.all? { |dr| !dr.result.changed? || dr.result.save }
schedule_result.dependent_results.all? { |dr| !dr.result.changed? || dr.result.save(validate: false) }
end || false
schedule_result.success = save_result

@ -101,6 +101,7 @@ class WorkPackages::ScheduleDependency
descendants + WorkPackage
.with_ancestor(descendants)
.includes(follows_relations: :to)
.where.not(id: known_work_packages_by_id.keys)
end

@ -90,7 +90,7 @@ class WorkPackages::SetAttributesService < ::BaseServices::SetAttributes
elsif parent_start_earlier_than_due?
work_package.parent.start_date
elsif Setting.work_package_startdate_is_adddate?
Date.today
Time.zone.today
end
end
@ -111,7 +111,7 @@ class WorkPackages::SetAttributesService < ::BaseServices::SetAttributes
# And the new type has a default text
default_description = work_package.type&.description
return unless default_description.present?
return if default_description.blank?
# And the current description matches ANY current default text
return unless work_package.description.blank? || default_description?
@ -155,6 +155,7 @@ class WorkPackages::SetAttributesService < ::BaseServices::SetAttributes
model.change_by_system do
set_version_to_nil
reassign_category
set_parent_to_nil
reassign_type unless work_package.type_id_changed?
end
@ -194,6 +195,14 @@ class WorkPackages::SetAttributesService < ::BaseServices::SetAttributes
end
end
def set_parent_to_nil
if !Setting.cross_project_work_package_relations? &&
!work_package.parent_changed?
work_package.parent = nil
end
end
def reassign_category
# work_package is moved to another project
# reassign to the category with same name if any

@ -176,9 +176,9 @@ class WorkPackages::SetScheduleService
def date_rescheduling_delta(predecessor)
if predecessor.due_date.present?
predecessor.due_date - (predecessor.due_date_was || predecessor.due_date)
predecessor.due_date - (predecessor.due_date_before_last_save || predecessor.due_date_was || predecessor.due_date)
elsif predecessor.start_date.present?
predecessor.start_date - (predecessor.start_date_was || predecessor.start_date)
predecessor.start_date - (predecessor.start_date_before_last_save || predecessor.start_date_was || predecessor.start_date)
else
0
end

@ -26,97 +26,63 @@
# See COPYRIGHT and LICENSE files for more details.
#++
# TODO: use default update base class
class WorkPackages::UpdateService < ::BaseServices::BaseCallable
class WorkPackages::UpdateService < ::BaseServices::Update
include ::WorkPackages::Shared::UpdateAncestors
include ::Shared::ServiceContext
attr_accessor :user,
:model,
:contract_class
def initialize(user:, model:, contract_class: WorkPackages::UpdateContract)
self.user = user
self.model = model
self.contract_class = contract_class
end
def perform(send_notifications: true, **attributes)
in_context(model, send_notifications) do
update(attributes)
end
end
include Attachments::ReplaceAttachments
private
def update(attributes)
result = set_attributes(attributes)
def after_perform(service_call)
update_related_work_packages(service_call)
cleanup(service_call.result)
if result.success?
work_package.attachments = work_package.attachments_replacements if work_package.attachments_replacements
result.merge!(update_dependent)
service_call
end
if save_if_valid(result)
update_ancestors([work_package]).each do |ancestor_result|
result.merge!(ancestor_result)
end
def update_related_work_packages(service_call)
update_ancestors([service_call.result]).each do |ancestor_service_call|
service_call.merge!(ancestor_service_call)
end
result
update_related(service_call.result).each do |related_service_call|
service_call.merge!(related_service_call)
end
def save_if_valid(result)
if result.success?
result.success = consolidated_results(result)
.all?(&:save)
end
result.success?
def update_related(work_package)
consolidated_calls(update_descendants(work_package) + reschedule_related(work_package))
.reject { |dependent_call| dependent_call.result.id == work_package.id }
.each { |dependent_call| dependent_call.result.save(validate: false) }
end
def update_dependent
result = ServiceResult.new(success: true, result: work_package)
result.merge!(update_descendants)
cleanup if result.success?
result.merge!(reschedule_related)
def update_descendants(work_package)
if work_package.saved_change_to_project_id?
attributes = { project: work_package.project }
result
work_package.descendants.map do |descendant|
set_descendant_attributes(attributes, descendant)
end
else
[]
end
end
def set_attributes(attributes, wp = work_package)
def set_descendant_attributes(attributes, descendant)
WorkPackages::SetAttributesService
.new(user:,
model: wp,
contract_class:)
model: descendant,
contract_class: WorkPackages::UpdateDependentContract)
.call(attributes)
end
def update_descendants
result = ServiceResult.new(success: true, result: work_package)
if work_package.project_id_changed?
attributes = { project: work_package.project }
work_package.descendants.each do |descendant|
result.add_dependent!(set_attributes(attributes, descendant))
end
end
result
end
def cleanup
if work_package.project_id_changed?
def cleanup(work_package)
if work_package.saved_change_to_project_id?
moved_work_packages = [work_package] + work_package.descendants
delete_relations(moved_work_packages)
move_time_entries(moved_work_packages, work_package.project_id)
end
if work_package.type_id_changed?
reset_custom_values
if work_package.saved_change_to_type_id?
reset_custom_values(work_package)
end
end
@ -134,60 +100,29 @@ class WorkPackages::UpdateService < ::BaseServices::BaseCallable
.update_all(project_id:)
end
def reset_custom_values
def reset_custom_values(work_package)
work_package.reset_custom_values!
end
def reschedule_related
result = ServiceResult.new(success: true, result: work_package)
with_temporarily_persisted_parent_changes do
if work_package.parent_id_changed? && work_package.parent_id_was
result.merge!(reschedule_former_siblings)
end
result.merge!(reschedule(work_package))
end
result
def reschedule_related(work_package)
rescheduled = if work_package.saved_change_to_parent_id? && work_package.parent_id_before_last_save
reschedule_former_siblings(work_package).dependent_results
else
[]
end
def with_temporarily_persisted_parent_changes
# Explicitly using requires_new: true since we are already within a transaction.
# Because of that, raising ActiveRecord::Rollback would have no effect:
# https://www.bigbinary.com/learn-rubyonrails-book/activerecord-transactions-in-depth#nested-transactions
WorkPackage.transaction(requires_new: true) do
if work_package.parent_id_changed?
# HACK: we need to persist the parent relation before rescheduling the parent
# and the former parent since we rely on the database for scheduling.
# The following will update the parent_id of the work package without that being noticed by the
# work package instance (work_package) that is already instantiated. That way, the change can be rolled
# back without any side effects to the instance (e.g. dirty tracking).
WorkPackage.where(id: work_package.id).update_all(parent_id: work_package.parent_id)
work_package.rebuild! # using the ClosureTree#rebuild! method to update the transitive hierarchy information
rescheduled + reschedule(work_package, [work_package]).dependent_results
end
yield
# Always rolling back the changes we made in here
raise ActiveRecord::Rollback
end
def reschedule_former_siblings(work_package)
reschedule(work_package, WorkPackage.where(parent_id: work_package.parent_id_before_last_save))
end
# Rescheduling the former siblings will lead to the whole former tree being rescheduled.
def reschedule_former_siblings
reschedule(WorkPackage.where(parent_id: work_package.parent_id_was))
end
def reschedule(work_packages)
def reschedule(work_package, work_packages)
WorkPackages::SetScheduleService
.new(user:,
work_package: work_packages)
.call(changed_attributes)
end
def changed_attributes
work_package.changed.map(&:to_sym)
.call(work_package.saved_changes.keys.map(&:to_sym))
end
# When multiple services change a work package, we still only want one update to the database due to:
@ -195,19 +130,15 @@ class WorkPackages::UpdateService < ::BaseServices::BaseCallable
# * having only one journal entry
# * stale object errors
# we thus consolidate the results so that one instance contains the changes made by all the services.
def consolidated_results(result)
result.all_results.group_by(&:id).inject([]) do |a, (_, instances)|
master = instances.pop
instances.each do |instance|
master.attributes = instance.changes.transform_values(&:last)
def consolidated_calls(service_calls)
service_calls
.group_by { |sc| sc.result.id }
.map do |(_, same_work_package_calls)|
same_work_package_calls.pop.tap do |master|
same_work_package_calls.each do |sc|
master.result.attributes = sc.result.changes.transform_values(&:last)
end
a + [master]
end
end
def work_package
model
end
end

@ -179,6 +179,8 @@ module Redmine
end
def ensure_custom_values_complete
return unless custom_values.loaded? && (custom_values.any?(&:changed?) || custom_value_destroyed)
self.custom_values = custom_field_values
end

@ -81,8 +81,10 @@ module OpenProject::Backlogs::List
protected
def assume_bottom_position
update_columns(position: bottom_position_in_list(self).to_i + 1)
# Override acts_as_list implementation to avoid it calling save.
# Calling save would remove the changes/saved_changes information.
def set_list_position(new_position, _raise_exception_if_save_fails = false) # rubocop:disable Style/OptionalBooleanParameter
update_columns(position: new_position)
end
def fix_other_work_package_positions

@ -32,37 +32,45 @@ module OpenProject::Backlogs::Patches::UpdateServicePatch
end
module InstanceMethods
def update_descendants
def update_descendants(work_package)
super_result = super
if work_package.in_backlogs_type? && work_package.version_id_changed?
inherit_version_to_descendants(super_result)
if work_package.in_backlogs_type? && work_package.saved_change_to_version_id?
super_result += inherit_version_to_descendants(work_package)
end
super_result
end
def inherit_version_to_descendants(result)
all_descendants = work_package
def inherit_version_to_descendants(work_package)
all_descendants = sorted_descendants(work_package)
descendant_tasks = descendant_tasks_of(all_descendants)
attributes = { version_id: work_package.version_id }
descendant_tasks.map do |task|
# Ensure the parent is already moved to new version so that validation errors are avoided.
task.parent = ([work_package] + all_descendants).detect { |d| d.id == task.parent_id }
set_descendant_attributes(attributes, task)
end
end
def sorted_descendants(work_package)
work_package
.descendants
.includes(project: :enabled_modules)
.order_by_ancestors('asc')
.select('work_packages.*')
end
def descendant_tasks_of(descendants)
stop_descendants_ids = []
descendant_tasks = all_descendants.reject do |t|
descendants.reject do |t|
if stop_descendants_ids.include?(t.parent_id) || !t.is_task?
stop_descendants_ids << t.id
end
end
attributes = { version_id: work_package.version_id }
descendant_tasks.each do |task|
# Ensure the parent is already moved to new version so that validation errors are avoided.
task.parent = ([work_package] + all_descendants).detect { |d| d.id == task.parent_id }
result.add_dependent!(set_attributes(attributes, task))
end
end
end
end

@ -1,198 +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.
#++
require 'spec_helper'
require 'rack/test'
describe 'API v3 Work package resource', type: :request, content_type: :json do
include Rack::Test::Methods
include Capybara::RSpecMatchers
include API::V3::Utilities::PathHelper
let(:work_package) do
create(
:work_package,
project_id: project.id,
parent:,
subject: "Updated WorkPackage"
)
end
let!(:parent) do
create(:work_package, project_id: project.id, type:, subject: "Invalid Dependent WorkPackage").tap do |parent|
parent.custom_values.create custom_field: custom_field, value: custom_field.possible_values.first.id
cv = parent.custom_values.last
cv.update_column :value, "0"
end
end
let(:project) do
create(:project, identifier: 'deperr', public: false).tap do |project|
project.types << type
end
end
let(:type) do
create(:type).tap do |type|
type.custom_fields << custom_field
end
end
let(:status) { create :status }
let(:custom_field) do
create(
:list_wp_custom_field,
name: "Gate",
possible_values: %w(A B C),
is_required: true
)
end
let(:role) { create(:role, permissions:) }
let(:permissions) { %i[view_work_packages edit_work_packages create_work_packages] }
let(:current_user) do
create(:user, member_in_project: project, member_through_role: role)
end
let(:dependent_error_result) do
proc do |instance, _attributes, work_package|
result = ServiceResult.new(success: true, result: (instance.respond_to?(:model) && instance.model) || work_package)
dep = parent
dep.errors.add :base, "invalid", message: "invalid"
result.add_dependent!(ServiceResult.new(success: false, errors: dep.errors, result: dep))
result
end
end
before do
login_as current_user
end
describe '#patch' do
let(:path) { api_v3_paths.work_package work_package.id }
let(:valid_params) do
{
_type: 'WorkPackage',
lockVersion: work_package.lock_version
}
end
subject(:response) { last_response }
shared_context 'patch request' do
before do
patch path, params.to_json, 'CONTENT_TYPE' => 'application/json'
end
end
before do
allow_any_instance_of(WorkPackages::UpdateService).to receive(:update_dependent, &dependent_error_result)
end
context 'attribute' do
let(:params) { valid_params.merge(startDate: "2018-05-23") }
include_context 'patch request'
it { expect(response.status).to eq(422) }
it 'responds with an error' do
expected_error = {
_type: "Error",
errorIdentifier: "urn:openproject-org:api:v3:errors:PropertyConstraintViolation",
message: "Error attempting to alter dependent object: Work package ##{parent.id} - #{parent.subject}: invalid",
_embedded: {
details: {
attribute: "base"
}
}
}
expect(subject.body).to be_json_eql(expected_error.to_json)
end
end
end
describe '#post' do
let(:current_user) { create :admin }
let(:path) { api_v3_paths.work_packages }
let(:valid_params) do
{
_type: 'WorkPackage',
lockVersion: 0,
_links: {
author: { href: "/api/v3/users/#{current_user.id}" },
project: { href: "/api/v3/projects/#{project.id}" },
status: { href: "/api/v3/statuses/#{status.id}" },
priority: { href: "/api/v3/priorities/#{work_package.priority.id}" }
}
}
end
subject(:response) { last_response }
shared_context 'post request' do
before do
post path, params.to_json, 'CONTENT_TYPE' => 'application/json'
end
end
before do
allow_any_instance_of(WorkPackages::CreateService).to receive(:create, &dependent_error_result)
end
context 'request' do
let(:params) { valid_params.merge(subject: "Test Subject") }
include_context 'post request'
it { expect(response.status).to eq(422) }
it 'responds with an error' do
expected_error = {
_type: "Error",
errorIdentifier: "urn:openproject-org:api:v3:errors:PropertyConstraintViolation",
message: "Error attempting to alter dependent object: Work package ##{parent.id} - #{parent.subject}: invalid",
_embedded: {
details: {
attribute: "base"
}
}
}
expect(subject.body).to be_json_eql(expected_error.to_json)
end
end
end
end

@ -1096,6 +1096,33 @@ describe WorkPackages::SetAttributesService, type: :model do
end
end
end
context 'for parent' do
let(:parent_work_package) { build_stubbed(:work_package, project:) }
let(:work_package) do
build_stubbed(:work_package, project:, type: initial_type, parent: parent_work_package)
end
context 'with cross project relations allowed', with_settings: { cross_project_work_package_relations: true } do
it 'keeps the parent' do
expect(subject)
.to be_success
expect(work_package.parent)
.to eql(parent_work_package)
end
end
context 'with cross project relations disabled', with_settings: { cross_project_work_package_relations: false } do
it 'deletes the parent' do
expect(subject)
.to be_success
expect(work_package.parent)
.to be_nil
end
end
end
end
context 'when updating project before calling the service' do

@ -105,7 +105,6 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
instance.call(**attributes.merge(send_notifications: false).symbolize_keys)
end
describe '#call' do
describe 'updating subject' do
let(:attributes) { { subject: 'New subject' } }
@ -118,7 +117,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
end
describe 'updating project' do
context 'when updating the project' do
let(:target_project) do
p = create(:project,
types: target_types,
@ -136,7 +135,15 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:target_parent) { nil }
let(:target_types) { [type] }
describe 'with missing permissions' do
it 'is is success and updates the project' do
expect(subject)
.to be_success
expect(work_package.reload.project)
.to eql target_project
end
context 'with missing permissions' do
let(:target_permissions) { [] }
it 'is failure' do
@ -162,6 +169,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
expect(TimeEntry.where(id: time_entries.map(&:id)).pluck(:project_id).uniq)
.to match_array [target_project.id]
end
end
describe 'categories' do
let(:category) do
@ -190,7 +198,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
end
context 'w/o target category' do
context 'without a target category' do
let!(:other_category) do
create(:category,
project: target_project)
@ -220,7 +228,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
project:)
end
context 'unshared version' do
context 'with an unshared version' do
it 'removes the version' do
expect(subject)
.to be_success
@ -230,7 +238,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
end
context 'system wide shared version' do
context 'with a system wide shared version' do
let(:sharing) { 'system' }
it 'keeps the version' do
@ -242,12 +250,12 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
end
context 'move work package in project hierarchy' do
context 'when moving the work package in project hierarchy' do
let(:target_parent) do
project
end
context 'unshared version' do
context 'with an unshared version' do
it 'removes the version' do
expect(subject)
.to be_success
@ -257,7 +265,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
end
context 'shared version' do
context 'with a shared version' do
let(:sharing) { 'tree' }
it 'keeps the version' do
@ -331,18 +339,46 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
end
end
describe 'relations' do
let!(:relation) do
create(:follows_relation,
from: work_package,
to: create(:work_package,
project:))
end
context 'with cross project relations allowed', with_settings: { cross_project_work_package_relations: true } do
it 'keeps the relation' do
expect(subject)
.to be_success
expect(Relation.find_by(id: relation.id))
.to eql(relation)
end
end
context 'with cross project relations disabled', with_settings: { cross_project_work_package_relations: false } do
it 'deletes the relation' do
expect(subject)
.to be_success
expect(Relation.find_by(id: relation.id))
.to be_nil
end
end
end
end
describe 'inheriting dates' do
let(:attributes) { { start_date: Date.today - 8.days, due_date: Date.today + 12.days } }
let(:attributes) { { start_date: Time.zone.today - 8.days, due_date: Time.zone.today + 12.days } }
let(:sibling1_attributes) do
work_package_attributes.merge(start_date: Date.today - 5.days,
due_date: Date.today + 10.days,
work_package_attributes.merge(start_date: Time.zone.today - 5.days,
due_date: Time.zone.today + 10.days,
parent: parent_work_package)
end
let(:sibling2_attributes) do
work_package_attributes.merge(due_date: Date.today + 16.days,
work_package_attributes.merge(due_date: Time.zone.today + 16.days,
parent: parent_work_package)
end
@ -561,18 +597,18 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
author_id: user.id,
status_id: status.id,
priority:,
start_date: Date.today,
due_date: Date.today + 5.days }
start_date: Time.zone.today,
due_date: Time.zone.today + 5.days }
end
let(:attributes) do
{ start_date: Date.today + 5.days,
due_date: Date.today + 10.days }
{ start_date: Time.zone.today + 5.days,
due_date: Time.zone.today + 10.days }
end
let(:following_attributes) do
work_package_attributes.merge(parent: following_parent_work_package,
subject: 'following',
start_date: Date.today + 6.days,
due_date: Date.today + 20.days)
start_date: Time.zone.today + 6.days,
due_date: Time.zone.today + 20.days)
end
let(:following_work_package) do
create(:work_package,
@ -582,8 +618,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:following_parent_attributes) do
work_package_attributes.merge(subject: 'following_parent',
start_date: Date.today + 6.days,
due_date: Date.today + 20.days)
start_date: Time.zone.today + 6.days,
due_date: Time.zone.today + 20.days)
end
let(:following_parent_work_package) do
create(:work_package,
@ -592,8 +628,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:following2_attributes) do
work_package_attributes.merge(parent: following2_parent_work_package,
subject: 'following2',
start_date: Date.today + 21.days,
due_date: Date.today + 25.days)
start_date: Time.zone.today + 21.days,
due_date: Time.zone.today + 25.days)
end
let(:following2_work_package) do
create(:work_package,
@ -601,8 +637,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:following2_parent_attributes) do
work_package_attributes.merge(subject: 'following2_parent',
start_date: Date.today + 21.days,
due_date: Date.today + 25.days)
start_date: Time.zone.today + 21.days,
due_date: Time.zone.today + 25.days)
end
let(:following2_parent_work_package) do
create(:work_package,
@ -613,8 +649,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:following3_attributes) do
work_package_attributes.merge(subject: 'following3',
parent: following3_parent_work_package,
start_date: Date.today + 26.days,
due_date: Date.today + 30.days)
start_date: Time.zone.today + 26.days,
due_date: Time.zone.today + 30.days)
end
let(:following3_work_package) do
create(:work_package,
@ -624,8 +660,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:following3_parent_attributes) do
work_package_attributes.merge(subject: 'following3_parent',
start_date: Date.today + 26.days,
due_date: Date.today + 36.days)
start_date: Time.zone.today + 26.days,
due_date: Time.zone.today + 36.days)
end
let(:following3_parent_work_package) do
create(:work_package,
@ -634,8 +670,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:following3_sibling_attributes) do
work_package_attributes.merge(parent: following3_parent_work_package,
subject: 'following3_sibling',
start_date: Date.today + 32.days,
due_date: Date.today + 36.days)
start_date: Time.zone.today + 32.days,
due_date: Time.zone.today + 36.days)
end
let(:following3_sibling_work_package) do
create(:work_package,
@ -653,66 +689,63 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
following3_sibling_work_package
end
# rubocop:disable RSpec/ExampleLength
# rubocop:disable RSpec/MultipleExpectations
it 'propagates the changes to start/finish date along' do
expect(subject)
.to be_success
work_package.reload(select: %i(start_date due_date))
expect(work_package.start_date)
.to eql Date.today + 5.days
.to eql Time.zone.today + 5.days
expect(work_package.due_date)
.to eql Date.today + 10.days
.to eql Time.zone.today + 10.days
following_work_package.reload(select: %i(start_date due_date))
expect(following_work_package.start_date)
.to eql Date.today + 11.days
.to eql Time.zone.today + 11.days
expect(following_work_package.due_date)
.to eql Date.today + 25.days
.to eql Time.zone.today + 25.days
following_parent_work_package.reload(select: %i(start_date due_date))
expect(following_parent_work_package.start_date)
.to eql Date.today + 11.days
.to eql Time.zone.today + 11.days
expect(following_parent_work_package.due_date)
.to eql Date.today + 25.days
.to eql Time.zone.today + 25.days
following2_parent_work_package.reload(select: %i(start_date due_date))
expect(following2_parent_work_package.start_date)
.to eql Date.today + 26.days
.to eql Time.zone.today + 26.days
expect(following2_parent_work_package.due_date)
.to eql Date.today + 30.days
.to eql Time.zone.today + 30.days
following2_work_package.reload(select: %i(start_date due_date))
expect(following2_work_package.start_date)
.to eql Date.today + 26.days
.to eql Time.zone.today + 26.days
expect(following2_work_package.due_date)
.to eql Date.today + 30.days
.to eql Time.zone.today + 30.days
following3_work_package.reload(select: %i(start_date due_date))
expect(following3_work_package.start_date)
.to eql Date.today + 31.days
.to eql Time.zone.today + 31.days
expect(following3_work_package.due_date)
.to eql Date.today + 35.days
.to eql Time.zone.today + 35.days
following3_parent_work_package.reload(select: %i(start_date due_date))
expect(following3_parent_work_package.start_date)
.to eql Date.today + 31.days
.to eql Time.zone.today + 31.days
expect(following3_parent_work_package.due_date)
.to eql Date.today + 36.days
.to eql Time.zone.today + 36.days
following3_sibling_work_package.reload(select: %i(start_date due_date))
expect(following3_sibling_work_package.start_date)
.to eql Date.today + 32.days
.to eql Time.zone.today + 32.days
expect(following3_sibling_work_package.due_date)
.to eql Date.today + 36.days
.to eql Time.zone.today + 36.days
end
# rubocop:enable RSpec/ExampleLength
# rubocop:enable RSpec/MultipleExpectations
end
describe 'rescheduling work packages forward follows/hierarchy relations' do
@ -734,18 +767,20 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
author_id: user.id,
status_id: status.id,
priority:,
start_date: Date.today,
due_date: Date.today + 5.days }
start_date: Time.zone.today,
due_date: Time.zone.today + 5.days }
end
let(:attributes) do
{ start_date: Date.today - 5.days,
due_date: Date.today }
{
start_date: Time.zone.today - 5.days,
due_date: Time.zone.today
}
end
let(:following_attributes) do
work_package_attributes.merge(parent: following_parent_work_package,
subject: 'following',
start_date: Date.today + 6.days,
due_date: Date.today + 20.days)
start_date: Time.zone.today + 6.days,
due_date: Time.zone.today + 20.days)
end
let(:following_work_package) do
create(:work_package,
@ -755,8 +790,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:following_parent_attributes) do
work_package_attributes.merge(subject: 'following_parent',
start_date: Date.today + 6.days,
due_date: Date.today + 20.days)
start_date: Time.zone.today + 6.days,
due_date: Time.zone.today + 20.days)
end
let(:following_parent_work_package) do
create(:work_package,
@ -764,8 +799,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:other_attributes) do
work_package_attributes.merge(subject: 'other',
start_date: Date.today + 10.days,
due_date: Date.today + 18.days)
start_date: Time.zone.today + 10.days,
due_date: Time.zone.today + 18.days)
end
let(:other_work_package) do
create(:work_package,
@ -774,8 +809,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:following2_attributes) do
work_package_attributes.merge(parent: following2_parent_work_package,
subject: 'following2',
start_date: Date.today + 24.days,
due_date: Date.today + 28.days)
start_date: Time.zone.today + 24.days,
due_date: Time.zone.today + 28.days)
end
let(:following2_work_package) do
create(:work_package,
@ -783,8 +818,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:following2_parent_attributes) do
work_package_attributes.merge(subject: 'following2_parent',
start_date: Date.today + 24.days,
due_date: Date.today + 28.days)
start_date: Time.zone.today + 24.days,
due_date: Time.zone.today + 28.days)
end
let(:following2_parent_work_package) do
following2 = create(:work_package,
@ -802,8 +837,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
end
let(:following3_attributes) do
work_package_attributes.merge(subject: 'following3',
start_date: Date.today + 29.days,
due_date: Date.today + 33.days)
start_date: Time.zone.today + 29.days,
due_date: Time.zone.today + 33.days)
end
let(:following3_work_package) do
create(:work_package,
@ -822,52 +857,48 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
following3_work_package
end
# rubocop:disable RSpec/ExampleLength
it 'propagates the changes to start/finish date along' do
expect(subject)
.to be_success
work_package.reload(select: %i(start_date due_date))
expect(work_package.start_date)
.to eql Date.today - 5.days
.to eql Time.zone.today - 5.days
expect(work_package.due_date)
.to eql Date.today
.to eql Time.zone.today
following_work_package.reload(select: %i(start_date due_date))
expect(following_work_package.start_date)
.to eql Date.today + 1.day
.to eql Time.zone.today + 1.day
expect(following_work_package.due_date)
.to eql Date.today + 15.days
.to eql Time.zone.today + 15.days
following_parent_work_package.reload(select: %i(start_date due_date))
expect(following_parent_work_package.start_date)
.to eql Date.today + 1.day
.to eql Time.zone.today + 1.day
expect(following_parent_work_package.due_date)
.to eql Date.today + 15.days
.to eql Time.zone.today + 15.days
following2_parent_work_package.reload(select: %i(start_date due_date))
expect(following2_parent_work_package.start_date)
.to eql Date.today + 22.days
.to eql Time.zone.today + 22.days
expect(following2_parent_work_package.due_date)
.to eql Date.today + 26.days
.to eql Time.zone.today + 26.days
following2_work_package.reload(select: %i(start_date due_date))
expect(following2_work_package.start_date)
.to eql Date.today + 22.days
.to eql Time.zone.today + 22.days
expect(following2_work_package.due_date)
.to eql Date.today + 26.days
.to eql Time.zone.today + 26.days
following3_work_package.reload(select: %i(start_date due_date))
expect(following3_work_package.start_date)
.to eql Date.today + 27.days
.to eql Time.zone.today + 27.days
expect(following3_work_package.due_date)
.to eql Date.today + 31.days
.to eql Time.zone.today + 31.days
end
# rubocop:enable RSpec/ExampleLength
end
describe 'changing the parent' do
@ -879,8 +910,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
author_id: user.id,
status_id: status.id,
priority:,
start_date: Date.today + 3.days,
due_date: Date.today + 9.days
start_date: Time.zone.today + 3.days,
due_date: Time.zone.today + 9.days
}
end
let(:attributes) { { parent: new_parent_work_package } }
@ -892,8 +923,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
work_package_attributes.merge(
subject: 'former sibling',
parent: former_parent_work_package,
start_date: Date.today + 3.days,
due_date: Date.today + 6.days
start_date: Time.zone.today + 3.days,
due_date: Time.zone.today + 6.days
)
end
let(:former_sibling_work_package) do
@ -907,16 +938,16 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
status_id: status.id,
priority:,
parent: former_parent_work_package,
start_date: Date.today + 7.days,
due_date: Date.today + 9.days }
start_date: Time.zone.today + 7.days,
due_date: Time.zone.today + 9.days }
end
let(:new_parent_attributes) do
work_package_attributes.merge(
subject: 'new parent',
parent: nil,
start_date: Date.today + 10.days,
due_date: Date.today + 12.days
start_date: Time.zone.today + 10.days,
due_date: Time.zone.today + 12.days
)
end
let(:new_parent_work_package) do
@ -927,8 +958,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
work_package_attributes.merge(
subject: 'new sibling',
parent: new_parent_work_package,
start_date: Date.today + 10.days,
due_date: Date.today + 12.days
start_date: Time.zone.today + 10.days,
due_date: Time.zone.today + 12.days
)
end
let(:new_sibling_work_package) do
@ -978,8 +1009,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
work_package_attributes.merge(
subject: 'new parent',
parent: nil,
start_date: Date.today + 8.days,
due_date: Date.today + 14.days
start_date: Time.zone.today + 8.days,
due_date: Time.zone.today + 14.days
)
end
let(:attributes) { { parent: new_parent_work_package } }
@ -991,8 +1022,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
work_package_attributes.merge(
subject: 'new parent predecessor',
parent: nil,
start_date: Date.today + 1.day,
due_date: Date.today + 4.days
start_date: Time.zone.today + 1.day,
due_date: Time.zone.today + 4.days
)
end
let(:new_parent_predecessor_work_package) do
@ -1007,8 +1038,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
author_id: user.id,
status_id: status.id,
priority:,
start_date: Date.today,
due_date: Date.today + 3.days }
start_date: Time.zone.today,
due_date: Time.zone.today + 3.days }
end
before do
@ -1057,8 +1088,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
status_id: status.id,
priority:,
parent: parent_work_package,
start_date: Date.today,
due_date: Date.today + 3.days }
start_date: Time.zone.today,
due_date: Time.zone.today + 3.days }
end
let(:attributes) { { parent: nil } }
@ -1069,8 +1100,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
author_id: user.id,
status_id: status.id,
priority:,
start_date: Date.today,
due_date: Date.today + 10.days }
start_date: Time.zone.today,
due_date: Time.zone.today + 10.days }
end
let(:parent_work_package) do
@ -1080,8 +1111,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:sibling_attributes) do
work_package_attributes.merge(
subject: 'sibling',
start_date: Date.today + 4.days,
due_date: Date.today + 10.days
start_date: Time.zone.today + 4.days,
due_date: Time.zone.today + 10.days
)
end
@ -1129,6 +1160,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
create(:attachment, container: nil, author: user)
end
# rubocop:disable RSpec/ExampleLength
it 'reports on invalid attachments and replaces the existent with the new if everything is valid' do
work_package.attachments.reload
@ -1171,7 +1203,7 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
expect(Attachment.all)
.to match_array [other_users_attachment]
end
end
# rubocop:enable RSpec/ExampleLength
end
##
@ -1205,27 +1237,145 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model, with_ma
let(:project_types) { [type, new_type] }
let(:attributes) { { type: new_type } }
context 'work package does NOT have default status' do
context 'when the work package does NOT have default status' do
let(:status) { create(:status) }
it 'assigns the default status' do
expect(work_package).to receive(:status=).and_call_original
expect(subject).to be_success
expect(work_package.status).to eq(Status.default)
end
end
context 'work package does have default status' do
context 'when the work package does have default status' do
let(:status) { create :default_status }
let!(:workflow_type) do
create(:workflow, type: new_type, role:, old_status_id: status.id)
end
it 'does not set the status' do
expect(work_package).not_to receive(:status=)
expect(subject).to be_success
expect(work_package)
.not_to be_saved_change_to_status_id
end
end
end
describe 'removing an invalid parent' do
# The parent does not have a required custom field set but will need to be touched since.
# the dates, inherited from its children (and then the only remaining child) will have to be updated.
let!(:parent) do
create(:work_package,
type: project.types.first,
project:,
start_date: Time.zone.today - 1.day,
due_date: Time.zone.today + 5.days)
end
let!(:custom_field) do
create(:int_wp_custom_field, is_required: true, is_for_all: true, default_value: nil).tap do |cf|
project.types.first.custom_fields << cf
project.work_package_custom_fields << cf
end
end
let!(:sibling) do
create(:work_package,
type: project.types.first,
project:,
parent:,
start_date: Time.zone.today + 1.day,
due_date: Time.zone.today + 5.days,
"custom_field_#{custom_field.id}": 5)
end
let!(:attributes) { { parent: nil } }
let(:work_package_attributes) do
{
start_date: Time.zone.today - 1.day,
due_date: Time.zone.today + 1.day,
project:,
type: project.types.first,
parent:,
"custom_field_#{custom_field.id}": 8
}
end
it 'removes the parent successfully and reschedules the parent' do
expect(subject).to be_success
expect(work_package.reload.parent).to be_nil
expect(parent.reload.start_date)
.to eql(sibling.start_date)
expect(parent.due_date)
.to eql(sibling.due_date)
end
end
describe 'updating an invalid work package' do
# The work package does not have a required custom field set.
let(:custom_field) do
create(:int_wp_custom_field, is_required: true, is_for_all: true, default_value: nil).tap do |cf|
project.types.first.custom_fields << cf
project.work_package_custom_fields << cf
end
end
let(:attributes) { { subject: 'A new subject' } }
let(:work_package_attributes) do
{
subject: 'The old subject',
project:,
type: project.types.first
}
end
before do
# Creating the custom field after the work package is already saved.
work_package
custom_field
end
it 'is a failure and does not save the change' do
expect(subject).to be_failure
expect(work_package.reload.subject)
.to eql work_package_attributes[:subject]
end
end
describe 'updating the type (custom field resetting)' do
let(:project_types) { [type, new_type] }
let(:new_type) { create(:type) }
let!(:custom_field_of_current_type) do
create(:int_wp_custom_field, default_value: nil).tap do |cf|
type.custom_fields << cf
project.work_package_custom_fields << cf
end
end
let!(:custom_field_of_new_type) do
create(:int_wp_custom_field, default_value: 8).tap do |cf|
new_type.custom_fields << cf
project.work_package_custom_fields << cf
end
end
let(:attributes) do
{ type: new_type }
end
let(:work_package_attributes) do
{
type:,
project:,
"custom_field_#{custom_field_of_current_type.id}": 5
}
end
it 'is success, removes the existing custom field value and sets the default for the new one' do
expect(subject).to be_success
expect(work_package.reload.custom_values.pluck(:custom_field_id, :value))
.to eq [[custom_field_of_new_type.id, "8"]]
end
end
end

@ -27,238 +27,15 @@
#++
require 'spec_helper'
require 'services/base_services/behaves_like_update_service'
describe WorkPackages::UpdateService, type: :model do
let(:user) { build_stubbed(:user) }
let(:project) do
p = build_stubbed(:project)
allow(p).to receive(:shared_versions).and_return([])
p
end
let(:work_package) do
wp = build_stubbed(:work_package, project:)
wp.type = build_stubbed(:type)
wp.send(:clear_changes_information)
wp
end
let(:instance) do
described_class.new(user:,
model: work_package)
end
before do
# Stub update_ancestors because it messes with the jouralizing expectations
allow(instance).to receive(:update_ancestors).and_return []
end
describe 'call' do
let(:set_attributes_service) do
service = double("WorkPackages::SetAttributesService",
new: set_attributes_service_instance)
stub_const('WorkPackages::SetAttributesService', service)
service
end
let(:send_notifications) { true }
let(:set_attributes_service_instance) do
instance = double("WorkPackages::SetAttributesServiceInstance")
allow(instance)
.to receive(:call) do |attributes|
work_package.attributes = attributes
set_service_results
end
instance
end
let(:errors) { [] }
let(:set_service_results) { ServiceResult.new success: true, result: work_package }
let(:work_package_save_result) { true }
before do
set_attributes_service
end
before do
expect(Journal::NotificationConfiguration)
.to receive(:with)
.with(send_notifications)
.and_yield
allow(work_package)
.to receive(:save)
.and_return work_package_save_result
end
shared_examples_for 'service call' do
subject { instance.call(**call_attributes.merge(send_notifications:).symbolize_keys) }
it 'is successful' do
expect(subject.success?).to be_truthy
end
it 'sets the value' do
subject
attributes.each do |attribute, key|
expect(work_package.send(attribute)).to eql key
end
end
it 'has no errors' do
expect(subject.errors.all?(&:empty?)).to be_truthy
end
context 'when setting the attributes is unsuccessful (invalid)' do
let(:errors) { ActiveModel::Errors.new(work_package) }
let(:set_service_results) { ServiceResult.new success: false, errors:, result: work_package }
it 'is unsuccessful' do
expect(subject.success?).to be_falsey
end
it 'does not persist the changes' do
subject
expect(work_package).not_to receive(:save)
end
it 'exposes the errors' do
errors.add(:base, 'This is a custom error!')
subject
expect(subject.errors).to eql errors
expect(subject.errors[:base]).to include 'This is a custom error!'
end
end
context 'when the saving is unsuccessful' do
let(:work_package_save_result) { false }
let(:saving_errors) { ActiveModel::Errors.new(work_package) }
before do
allow(work_package)
.to receive(:errors)
.and_return(saving_errors)
end
it 'is unsuccessful' do
expect(subject.success?).to be_falsey
end
it 'leaves the value unchanged' do
subject
expect(work_package.changed?).to be_truthy
end
it 'exposes the errors, but is a new instance' do
saving_errors.add(:base, 'This is a custom error!')
subject
expect(subject.errors).not_to eql saving_errors
expect(subject.errors[:base]).to include 'This is a custom error!'
end
end
end
context 'update subject before calling the service' do
let(:call_attributes) { {} }
let(:attributes) { { subject: 'blubs blubs' } }
# This is now only a very basic test testing the structure of the service.
# The domain tests are in the update_service_integration_spec.rb
it_behaves_like 'BaseServices update service' do
before do
work_package.attributes = attributes
end
it_behaves_like 'service call'
end
context 'updating subject via attributes' do
let(:call_attributes) { attributes }
let(:attributes) { { subject: 'blubs blubs' } }
it_behaves_like 'service call'
end
context 'when switching the project' do
let(:target_project) do
build_stubbed(:project)
end
let(:call_attributes) { attributes }
let(:attributes) { { project: target_project } }
it_behaves_like 'service call'
context 'relations' do
let!(:scope) do
instance_double(ActiveRecord::Relation).tap do |relations|
allow(Relation)
.to receive(:of_work_package)
.with([work_package])
.and_return(relations)
allow(relations)
.to receive(:destroy_all)
end
end
it 'removes the relations if the setting does not permit cross project relations' do
allow(Setting)
.to receive(:cross_project_work_package_relations?)
.and_return false
instance.call(project: target_project)
expect(scope)
.to have_received(:destroy_all)
end
it 'leaves the relations unchanged if the setting allows cross project relations' do
allow(Setting)
.to receive(:cross_project_work_package_relations?)
.and_return true
instance.call(project: target_project)
expect(scope)
.not_to have_received(:destroy_all)
end
end
context 'time_entries' do
it 'moves the time entries' do
scope = double('scope')
expect(TimeEntry)
.to receive(:on_work_packages)
.with([work_package])
.and_return(scope)
expect(scope)
.to receive(:update_all)
.with(project_id: target_project.id)
instance.call(project: target_project)
end
end
end
context 'when switching the type' do
let(:target_type) { build_stubbed(:type) }
context 'custom_values' do
it 'resets the custom values' do
expect(work_package)
.to receive(:reset_custom_values!)
instance.call(type: target_type)
end
end
allow(set_attributes_errors)
.to receive(:merge!)
end
end
end

Loading…
Cancel
Save