Merge remote-tracking branch 'origin/dev' into feature/readonly-status-property

pull/10801/head
ulferts 2 years ago
commit 2711bb581c
No known key found for this signature in database
GPG Key ID: A205708DE1284017
  1. 9
      app/contracts/work_packages/base_contract.rb
  2. 36
      app/contracts/work_packages/update_dependent_contract.rb
  3. 13
      app/models/type/attributes.rb
  4. 10
      app/services/attachments/replace_attachments.rb
  5. 6
      app/services/base_services/base_contracted.rb
  6. 2
      app/services/base_services/copy.rb
  7. 4
      app/services/base_services/create.rb
  8. 3
      app/services/journals/create_service.rb
  9. 2
      app/services/relations/base_service.rb
  10. 1
      app/services/work_packages/schedule_dependency.rb
  11. 16
      app/services/work_packages/set_attributes_service.rb
  12. 4
      app/services/work_packages/set_schedule_service.rb
  13. 173
      app/services/work_packages/update_service.rb
  14. 3
      config/locales/en.yml
  15. 5
      db/migrate/20220614132200_add_ignore_non_working_days_to_work_packages.rb
  16. 10
      docs/api/apiv3/components/schemas/work_package_model.yml
  17. 2
      docs/api/apiv3/paths/file_link.yml
  18. 1
      docs/api/apiv3/paths/file_link_open.yml
  19. 42
      docs/api/apiv3/tags/work_packages.yml
  20. 6
      lib/api/v3/work_packages/schema/work_package_schema_representer.rb
  21. 8
      lib/api/v3/work_packages/work_package_payload_representer.rb
  22. 9
      lib/api/v3/work_packages/work_package_representer.rb
  23. 2
      lib_static/plugins/acts_as_customizable/lib/acts_as_customizable.rb
  24. 6
      modules/backlogs/lib/open_project/backlogs/list.rb
  25. 44
      modules/backlogs/lib/open_project/backlogs/patches/update_service_patch.rb
  26. 2
      modules/storages/spec/contracts/storages/file_links/create_contract_spec.rb
  27. 2
      modules/storages/spec/contracts/storages/file_links/delete_contract_spec.rb
  28. 2
      modules/storages/spec/contracts/storages/project_storages/create_contract_spec.rb
  29. 2
      modules/storages/spec/contracts/storages/project_storages/delete_contract_spec.rb
  30. 2
      modules/storages/spec/contracts/storages/storages/base_contract_spec.rb
  31. 2
      modules/storages/spec/contracts/storages/storages/create_contract_spec.rb
  32. 2
      modules/storages/spec/contracts/storages/storages/delete_contract_spec.rb
  33. 2
      modules/storages/spec/contracts/storages/storages/update_contract_spec.rb
  34. 2
      modules/storages/spec/features/admin_storages_spec.rb
  35. 2
      modules/storages/spec/features/create_and_delete_project_storage_spec.rb
  36. 2
      modules/storages/spec/features/delete_project_storage_and_file_links_spec.rb
  37. 13
      modules/storages/spec/features/show_file_links_spec.rb
  38. 2
      modules/storages/spec/permissions/manage_storage_in_project_spec.rb
  39. 12
      modules/storages/spec/requests/api/v3/file_links/file_links_spec.rb
  40. 4
      modules/storages/spec/requests/api/v3/storages/storages_spec.rb
  41. 2
      modules/storages/spec/requests/api/v3/work_packages/shared_filter_examples.rb
  42. 2
      modules/storages/spec/requests/api/v3/work_packages/work_packages_linkable_filter_spec.rb
  43. 2
      modules/storages/spec/requests/api/v3/work_packages/work_packages_linked_filter_spec.rb
  44. 2
      modules/storages/spec/services/storages/project_storages/delete_service_spec.rb
  45. 48
      modules/storages/spec/support/enable_storages_module.rb
  46. 37
      spec/contracts/work_packages/base_contract_spec.rb
  47. 55
      spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb
  48. 26
      spec/lib/api/v3/work_packages/work_package_payload_representer_spec.rb
  49. 41
      spec/lib/api/v3/work_packages/work_package_representer_spec.rb
  50. 12
      spec/models/queries/work_packages/columns/property_column_spec.rb
  51. 30
      spec/models/type_spec.rb
  52. 55
      spec/models/work_package_spec.rb
  53. 198
      spec/requests/api/v3/work_packages/dependent_errors_spec.rb
  54. 2
      spec/requests/oauth_clients/callback_flow_spec.rb
  55. 27
      spec/services/work_packages/set_attributes_service_spec.rb
  56. 1982
      spec/services/work_packages/update_service_integration_spec.rb
  57. 235
      spec/services/work_packages/update_service_spec.rb
  58. 47
      spec/support/shared/with_flag.rb

@ -80,6 +80,10 @@ module WorkPackages
end
attribute :schedule_manually
attribute :ignore_non_working_days,
writeable: ->(*) {
OpenProject::FeatureDecisions.work_packages_duration_field_active?
}
attribute :start_date,
writeable: ->(*) {
@ -98,7 +102,10 @@ module WorkPackages
model.leaf? || model.schedule_manually?
}
attribute :duration
attribute :duration,
writeable: ->(*) {
OpenProject::FeatureDecisions.work_packages_duration_field_active?
}
attribute :budget

@ -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

@ -32,15 +32,16 @@ module Type::Attributes
EXCLUDED = %w[_type
_dependencies
attribute_groups
links
parent_id
parent
description
schedule_manually
derived_start_date
derived_due_date
derived_estimated_time
duration].freeze
ignore_non_working_days
duration
description
links
parent_id
parent
schedule_manually].freeze
included do
# Allow plugins to define constraints

@ -34,13 +34,11 @@ module Attachments
private
def set_attributes(attributes)
call = super
if call.success? && call.result.attachments_replacements
call.result.attachments = call.result.attachments_replacements
super.tap do |call|
if call.success? && call.result.attachments_replacements
call.result.attachments = call.result.attachments_replacements
end
end
call
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
@ -303,6 +312,7 @@ class WorkPackages::SetAttributesService < ::BaseServices::SetAttributes
end
def date_changed_but_not_duration?
(work_package.start_date_changed? || work_package.due_date_changed?) && !work_package.duration_changed?
(work_package.start_date_changed? || work_package.due_date_changed? || work_package.duration.nil?) &&
!work_package.duration_changed?
end
end

@ -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
include Attachments::ReplaceAttachments
attr_accessor :user,
:model,
:contract_class
private
def initialize(user:, model:, contract_class: WorkPackages::UpdateContract)
self.user = user
self.model = model
self.contract_class = contract_class
end
def after_perform(service_call)
update_related_work_packages(service_call)
cleanup(service_call.result)
def perform(send_notifications: true, **attributes)
in_context(model, send_notifications) do
update(attributes)
end
service_call
end
private
def update(attributes)
result = set_attributes(attributes)
if result.success?
work_package.attachments = work_package.attachments_replacements if work_package.attachments_replacements
result.merge!(update_dependent)
def update_related_work_packages(service_call)
update_ancestors([service_call.result]).each do |ancestor_service_call|
service_call.merge!(ancestor_service_call)
end
if save_if_valid(result)
update_ancestors([work_package]).each do |ancestor_result|
result.merge!(ancestor_result)
end
update_related(service_call.result).each do |related_service_call|
service_call.merge!(related_service_call)
end
result
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
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
end
yield
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
# Always rolling back the changes we made in here
raise ActiveRecord::Rollback
end
rescheduled + reschedule(work_package, [work_package]).dependent_results
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))
def reschedule_former_siblings(work_package)
reschedule(work_package, WorkPackage.where(parent_id: work_package.parent_id_before_last_save))
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
end
a + [master]
end
end
def work_package
model
end
end

@ -581,7 +581,7 @@ en:
duration: "Duration"
end_insertion: "End of the insertion"
end_deletion: "End of the deletion"
version: "Version"
ignore_non_working_days: "Ignore non working days"
parent: "Parent"
parent_issue: "Parent"
parent_work_package: "Parent"
@ -594,6 +594,7 @@ en:
subproject: "Subproject"
time_entries: "Log time"
type: "Type"
version: "Version"
watcher: "Watcher"
'doorkeeper/application':
uid: "Client ID"

@ -0,0 +1,5 @@
class AddIgnoreNonWorkingDaysToWorkPackages < ActiveRecord::Migration[7.0]
def change
add_column :work_packages, :ignore_non_working_days, :boolean, default: true, null: false
end
end

@ -48,14 +48,14 @@ properties:
type: string
format: date
description: Similar to start date but is not set by a client but rather deduced
by the work packages's descendants. If manual scheduleManually is active, the
by the work packages' descendants. If manual scheduleManually is active, the
two dates can deviate.
readOnly: true
derivedDueDate:
type: string
format: date
description: Similar to due date but is not set by a client but rather deduced
by the work packages's descendants. If manual scheduleManually is active, the
by the work packages' descendants. If manual scheduleManually is active, the
two dates can deviate.
readOnly: true
duration:
@ -74,6 +74,12 @@ properties:
format: duration
description: Time a work package likely needs to be completed including its descendants
readOnly: true
ignoreNonWorkingDays:
type: boolean
description: |-
**(NOT IMPLEMENTED)** When scheduling, whether or not to ignore the non working days being defined.
A work package with the flag set to true will be allowed to be scheduled to a non working day.
readOnly: true
spentTime:
type: string
format: duration

@ -4,7 +4,6 @@ get:
summary: Gets a file link.
operationId: view_file_link
tags:
- Work Packages
- File links
description: |-
Gets a single file link resource of a work package.
@ -41,7 +40,6 @@ delete:
summary: Removes a file link.
operationId: delete_file_link
tags:
- Work Packages
- File links
description: |-
Removes a file link on a work package.

@ -4,7 +4,6 @@ get:
summary: Creates an opening uri of the linked file.
operationId: open_file_link
tags:
- Work Packages
- File links
description: |-
Creates a uri to open the origin file linked by the given file link. This uri depends on the storage type and

@ -44,26 +44,28 @@ description: |-
## Local Properties
| Property | Description | Type | Constraints | Supported operations | Condition |
| :--------------: | ------------------------------------------------------ | ----------- | ------------------------------------------------------------------------------------------------------ | -------------------- | -------------------------------- |
| id | Work package id | Integer | x > 0 | READ | |
| lockVersion | The version of the item as used for optimistic locking | Integer | | READ | |
| subject | Work package subject | String | not null; 1 <= length <= 255 | READ / WRITE | |
| type | Name of the work package's type | String | not null | READ | |
| description | The work package description | Formattable | | READ / WRITE | |
| scheduleManually | If false (default) schedule automatically. | Boolean | | READ / WRITE | |
| readonly | If true, the work package is in a readonly status so with the exception of the status, no other property can be altered. | Boolean | | READ | Enterprise edition only |
| startDate | Scheduled beginning of a work package | Date | Cannot be set for parent work packages; must be equal or greater than the earliest possible start date; Exists only on work packages of a non milestone type | READ / WRITE | |
| dueDate | Scheduled end of a work package | Date | Cannot be set for parent work packages; must be greater than or equal to the start date; Exists only on work packages of a non milestone type | READ / WRITE | |
| date | Date on which a milestone is achieved | Date | Exists only on work packages of a milestone type | READ / WRITE | |
| derivedStartDate | Similar to start date but is not set by a client but rather deduced by the work packages's descendants. If manual scheduleManually is active, the two dates can deviate. | Date | | READ | |
| derivedDueDate | Similar to due date but is not set by a client but rather deduced by the work packages's descendants. If manual scheduleManually is active, the two dates can deviate. | Date | | READ | |
| estimatedTime | Time a work package likely needs to be completed excluding its descendants | Duration | | READ / WRITE | |
| derivedEstimatedTime | Time a work package likely needs to be completed including its descendants | Duration | | READ | |
| spentTime | The time booked for this work package by users working on it | Duration | | READ | **Permission** view time entries |
| percentageDone | Amount of total completion for a work package | Integer | 0 <= x <= 100; Cannot be set for parent work packages | READ / WRITE | |
| createdAt | Time of creation | DateTime | | READ | |
| updatedAt | Time of the most recent change to the work package | DateTime | | READ | |
| Property | Description | Type | Constraints | Supported operations | Condition |
| :--------------: | ------------------------------------------------------ | ----------- | ------------------------------------------------------------------------------------------------------ | -------------------- | -------------------------------- |
| id | Work package id | Integer | x > 0 | READ | |
| lockVersion | The version of the item as used for optimistic locking | Integer | | READ | |
| subject | Work package subject | String | not null; 1 <= length <= 255 | READ / WRITE | |
| type | Name of the work package's type | String | not null | READ | |
| description | The work package description | Formattable | | READ / WRITE | |
| scheduleManually | If false (default) schedule automatically. | Boolean | | READ / WRITE | |
| startDate | Scheduled beginning of a work package | Date | Cannot be set for parent work packages; must be equal or greater than the earliest possible start date; Exists only on work packages of a non milestone type | READ / WRITE | |
| dueDate | Scheduled end of a work package | Date | Cannot be set for parent work packages; must be greater than or equal to the start date; Exists only on work packages of a non milestone type | READ / WRITE | |
| date | Date on which a milestone is achieved | Date | Exists only on work packages of a milestone type | READ / WRITE | |
| derivedStartDate | Similar to start date but is not set by a client but rather deduced by the work packages' descendants. If manual scheduleManually is active, the two dates can deviate. | Date | | READ | |
| derivedDueDate | Similar to due date but is not set by a client but rather deduced by the work packages' descendants. If manual scheduleManually is active, the two dates can deviate. | Date | | READ | |
| duration | The amount of time in hours the work package needs to be completed. | Duration | **(NOT IMPLEMENTED YET)** Not available for milestone type of work packages. | READ | |
| estimatedTime | Time a work package likely needs to be completed excluding its descendants | Duration | | READ / WRITE | |
| derivedEstimatedTime | Time a work package likely needs to be completed including its descendants | Duration | | READ | |
| ignoreNonWorkingDays | When scheduling, whether or not to ignore the non working days being defined. A work package with the flag set to true will be allowed to be scheduled to a non working day. | Boolean | **(NOT IMPLEMENTED YET)** | READ | |
| spentTime | The time booked for this work package by users working on it | Duration | | READ | **Permission** view time entries |
| percentageDone | Amount of total completion for a work package | Integer | 0 <= x <= 100; Cannot be set for parent work packages | READ / WRITE | |
| readonly | If true, the work package is in a readonly status so with the exception of the status, no other property can be altered. | Boolean | | READ | Enterprise edition only |
| createdAt | Time of creation | DateTime | | READ | |
| updatedAt | Time of the most recent change to the work package | DateTime | | READ | |
Note that the properties listed here only cover the built-in properties of the OpenProject Core.
Using plug-ins and custom fields a work package might contain various additional properties.

@ -128,6 +128,12 @@ module API
required: false,
has_default: true
schema :ignore_non_working_days,
type: 'Boolean',
required: false,
writable: false,
show_if: ->(*) { OpenProject::FeatureDecisions.work_packages_duration_field_active? }
schema :start_date,
type: 'Date',
required: false,

@ -36,13 +36,7 @@ module API
cached_representer disabled: true
def writeable_attributes
attributes = super
# TODO: Remove this, once the duration feature is complete
# We have to remove the duration field as it is not being accepted yet.
if OpenProject::FeatureDecisions.work_packages_duration_field_active?
attributes -= %w[duration]
end
attributes + %w[date]
super + %w[date]
end
def load_complete_model(model)

@ -393,8 +393,8 @@ module API
property :duration,
exec_context: :decorator,
skip_render: ->(represented:, **) {
represented.milestone? || !OpenProject::FeatureDecisions.work_packages_duration_field_active?
if: ->(represented:, **) {
!represented.milestone? && OpenProject::FeatureDecisions.work_packages_duration_field_active?
},
getter: ->(*) do
datetime_formatter.format_duration_from_hours(represented.duration_in_hours,
@ -402,6 +402,11 @@ module API
end,
render_nil: true
property :ignore_non_working_days,
if: ->(*) {
OpenProject::FeatureDecisions.work_packages_duration_field_active?
}
property :spent_time,
exec_context: :decorator,
getter: ->(*) do

@ -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,36 +32,44 @@ 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
.descendants
.includes(project: :enabled_modules)
.order_by_ancestors('asc')
.select('work_packages.*')
stop_descendants_ids = []
descendant_tasks = all_descendants.reject do |t|
if stop_descendants_ids.include?(t.parent_id) || !t.is_task?
stop_descendants_ids << t.id
end
end
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.each do |task|
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 }
result.add_dependent!(set_attributes(attributes, task))
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 = []
descendants.reject do |t|
if stop_descendants_ids.include?(t.parent_id) || !t.is_task?
stop_descendants_ids << t.id
end
end
end
end

@ -31,7 +31,7 @@ require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
require_relative 'shared_contract_examples'
describe Storages::FileLinks::CreateContract, :enable_storages do
describe Storages::FileLinks::CreateContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
it_behaves_like 'file_link contract' do

@ -30,7 +30,7 @@ require 'spec_helper'
require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
describe ::Storages::FileLinks::DeleteContract, :enable_storages do
describe ::Storages::FileLinks::DeleteContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
let(:current_user) { create(:user) }

@ -31,7 +31,7 @@ require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
require_relative 'shared_contract_examples'
describe Storages::ProjectStorages::CreateContract, :enable_storages do
describe Storages::ProjectStorages::CreateContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
it_behaves_like 'ProjectStorages contract' do

@ -30,7 +30,7 @@ require 'spec_helper'
require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
describe ::Storages::ProjectStorages::DeleteContract, :enable_storages do
describe ::Storages::ProjectStorages::DeleteContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
let(:current_user) { create(:user) }

@ -28,7 +28,7 @@
require_relative '../../../spec_helper'
describe Storages::Storages::BaseContract, :enable_storages, :storage_server_helpers, webmock: true do
describe Storages::Storages::BaseContract, :storage_server_helpers, with_flag: { storages_module_active: true }, webmock: true do
let(:current_user) { create(:admin) }
let(:storage_host) { 'https://host1.example.com' }
let(:storage) { build(:storage, host: storage_host) }

@ -31,7 +31,7 @@ require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
require_relative 'shared_contract_examples'
describe Storages::Storages::CreateContract, :enable_storages do
describe Storages::Storages::CreateContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
it_behaves_like 'storage contract' do

@ -30,7 +30,7 @@ require 'spec_helper'
require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
describe ::Storages::Storages::DeleteContract, :enable_storages do
describe ::Storages::Storages::DeleteContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
let(:storage) { create(:storage) }

@ -31,7 +31,7 @@ require_module_spec_helper
require 'contracts/shared/model_contract_shared_context'
require_relative 'shared_contract_examples'
describe ::Storages::Storages::UpdateContract, :enable_storages do
describe ::Storages::Storages::UpdateContract, with_flag: { storages_module_active: true } do
include_context 'ModelContract shared context'
it_behaves_like 'storage contract' do

@ -28,7 +28,7 @@
require_relative '../spec_helper'
describe 'Admin storages', :enable_storages, :storage_server_helpers, type: :feature, js: true do
describe 'Admin storages', :storage_server_helpers, with_flag: { storages_module_active: true }, type: :feature, js: true do
let(:admin) { create(:admin) }
before do

@ -31,7 +31,7 @@ require_relative '../spec_helper'
# Setup storages in Project -> Settings -> File Storages
# This tests assumes that a Storage has already been setup
# in the Admin section, tested by admin_storage_spec.rb.
describe 'Activation of storages in projects', :enable_storages, type: :feature, js: true do
describe 'Activation of storages in projects', with_flag: { storages_module_active: true }, type: :feature, js: true do
let(:user) { create(:user) }
# The first page is the Project -> Settings -> General page, so we need
# to provide the user with the edit_project permission in the role.

@ -30,7 +30,7 @@ require_relative '../spec_helper'
# Test if the deletion of a ProjectStorage actually deletes related FileLink
# objects.
describe 'Delete ProjectStorage with FileLinks', :enable_storages, type: :feature, js: true do
describe 'Delete ProjectStorage with FileLinks', with_flag: { storages_module_active: true }, type: :feature, js: true do
let(:user) { create(:user) }
let(:role) { create(:existing_role, permissions: [:manage_storages_in_project]) }
let(:project) do

@ -28,7 +28,7 @@
require_relative '../spec_helper'
describe 'Showing of file links in work package', :enable_storages, type: :feature, js: true do
describe 'Showing of file links in work package', with_flag: { storages_module_active: true }, type: :feature, js: true do
let(:permissions) { %i(view_work_packages edit_work_packages view_file_links) }
let(:project) { create(:project) }
let(:current_user) do
@ -41,6 +41,7 @@ describe 'Showing of file links in work package', :enable_storages, type: :featu
let(:wp_page) { ::Pages::FullWorkPackage.new(work_package, project) }
before do
project_storage
file_link
login_as current_user
@ -48,10 +49,6 @@ describe 'Showing of file links in work package', :enable_storages, type: :featu
end
context 'if work package has associated file links' do
before do
project_storage
end
it "must show associated file links" do
expect(page).to have_selector('[data-qa-selector="op-tab-content--tab-section"]', count: 2)
expect(page.find('[data-qa-selector="op-files-tab--file-link-list"]'))
@ -62,16 +59,14 @@ describe 'Showing of file links in work package', :enable_storages, type: :featu
context 'if user has no permission to see file links' do
let(:permissions) { %i(view_work_packages edit_work_packages) }
before do
project_storage
end
it 'must not show a file links section' do
expect(page).to have_selector('[data-qa-selector="op-tab-content--tab-section"]', count: 1)
end
end
context 'if project has no storage' do
let(:project_storage) { {} }
it 'must not show a file links section' do
expect(page).to have_selector('[data-qa-selector="op-tab-content--tab-section"]', count: 1)
end

@ -33,7 +33,7 @@ require_module_spec_helper
# rubocop:disable RSpec/EmptyExampleGroup
describe Storages::Admin::ProjectsStoragesController,
'manage_storage_in_project permission',
:enable_storages,
with_flag: { storages_module_active: true },
type: :controller do
include PermissionSpecs

@ -29,7 +29,7 @@
require 'spec_helper'
require_module_spec_helper
describe 'API v3 file links resource', :enable_storages, type: :request do
describe 'API v3 file links resource', with_flag: { storages_module_active: true }, type: :request do
include API::V3::Utilities::PathHelper
let(:permissions) { %i(view_work_packages view_file_links) }
@ -96,7 +96,7 @@ describe 'API v3 file links resource', :enable_storages, type: :request do
end
end
context 'if storages feature is inactive', :disable_storages do
context 'if storages feature is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'not found'
end
@ -187,7 +187,7 @@ describe 'API v3 file links resource', :enable_storages, type: :request do
end
end
context 'when storages module is inactive', :disable_storages do
context 'when storages module is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'not found'
end
end
@ -382,7 +382,7 @@ describe 'API v3 file links resource', :enable_storages, type: :request do
it_behaves_like 'not found'
end
context 'when storages module is inactive', :disable_storages do
context 'when storages module is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'not found'
end
end
@ -419,7 +419,7 @@ describe 'API v3 file links resource', :enable_storages, type: :request do
it_behaves_like 'not found'
end
context 'when storages module is inactive', :disable_storages do
context 'when storages module is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'not found'
end
end
@ -455,7 +455,7 @@ describe 'API v3 file links resource', :enable_storages, type: :request do
it_behaves_like 'not found'
end
context 'when storages module is inactive', :disable_storages do
context 'when storages module is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'not found'
end
end

@ -29,7 +29,7 @@
require 'spec_helper'
require_module_spec_helper
describe 'API v3 storages resource', :enable_storages, type: :request, content_type: :json do
describe 'API v3 storages resource', with_flag: { storages_module_active: true }, type: :request, content_type: :json do
include API::V3::Utilities::PathHelper
let(:permissions) { %i(view_file_links) }
@ -95,7 +95,7 @@ describe 'API v3 storages resource', :enable_storages, type: :request, content_t
it_behaves_like 'successful storage response'
end
context 'when storages module is inactive', :disable_storages do
context 'when storages module is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'not found'
end
end

@ -29,7 +29,7 @@
require 'spec_helper'
shared_examples_for 'filter unavailable when storages module is inactive' do
context 'when storages module is inactive', :disable_storages do
context 'when storages module is inactive', with_flag: { storages_module_active: false } do
it_behaves_like 'error response',
400,
'InvalidQuery',

@ -31,7 +31,7 @@ require_module_spec_helper
require_relative 'shared_filter_examples'
describe 'API v3 work packages resource with filters for the linkable to storage attribute',
:enable_storages,
with_flag: { storages_module_active: true },
type: :request,
content_type: :json do
include API::V3::Utilities::PathHelper

@ -31,7 +31,7 @@ require_module_spec_helper
require_relative 'shared_filter_examples'
describe 'API v3 work packages resource with filters for linked storage file',
:enable_storages,
with_flag: { storages_module_active: true },
type: :request,
content_type: :json do
include API::V3::Utilities::PathHelper

@ -30,7 +30,7 @@ require 'spec_helper'
require_module_spec_helper
require 'services/base_services/behaves_like_delete_service'
describe ::Storages::ProjectStorages::DeleteService, :enable_storages, type: :model do
describe ::Storages::ProjectStorages::DeleteService, with_flag: { storages_module_active: true }, type: :model do
context 'with records written to DB' do
let(:user) { create(:user) }
let(:role) { create(:existing_role, permissions: [:manage_storages_in_project]) }

@ -1,48 +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.
#++
RSpec.shared_context "with storages module enabled" do
let(:storages_module_active) { true }
before do
allow(OpenProject::FeatureDecisions).to receive(:storages_module_active?).and_return(storages_module_active)
end
end
RSpec.shared_context "with storages module disabled" do
let(:storages_module_active) { false }
end
RSpec.configure do |rspec|
# examples tagged with `:enable_storages` will automatically have context
# included and storages module enabled
rspec.include_context "with storages module enabled", :enable_storages
# examples tagged with `disable_storages` will automatically have context
# included and storages module disabled
rspec.include_context "with storages module disabled", :disable_storages
end

@ -476,7 +476,7 @@ describe WorkPackages::BaseContract do
end
end
describe 'duration' do
describe 'duration', with_flag: { work_packages_duration_field_active: true } do
context 'when setting the duration' do
before do
work_package.duration = 5
@ -485,6 +485,14 @@ describe WorkPackages::BaseContract do
it_behaves_like 'contract is valid'
end
context 'when setting the duration with the feature disabled', with_flag: { work_packages_duration_field_active: false } do
before do
work_package.duration = 5
end
it_behaves_like 'contract is invalid', duration: :error_readonly
end
context 'when setting the duration to 0' do
before do
work_package.duration = 0
@ -550,6 +558,33 @@ describe WorkPackages::BaseContract do
end
end
describe 'ignore_non_working_days' do
context 'when setting the value to true', with_flag: { work_packages_duration_field_active: true } do
before do
work_package.ignore_non_working_days = true
end
it_behaves_like 'contract is valid'
end
context 'when setting the value to false', with_flag: { work_packages_duration_field_active: true } do
before do
work_package.ignore_non_working_days = false
end
it_behaves_like 'contract is valid'
end
context 'when setting the value to false and with the feature disabled',
with_flag: { work_packages_duration_field_active: false } do
before do
work_package.ignore_non_working_days = false
end
it_behaves_like 'contract is invalid', ignore_non_working_days: :error_readonly
end
end
describe 'percentage done' do
it_behaves_like 'a parent unwritable property', :done_ratio

@ -293,15 +293,13 @@ describe ::API::V3::WorkPackages::Schema::WorkPackageSchemaRepresenter do
end
end
describe 'duration' do
describe 'duration', with_flag: { work_packages_duration_field_active: true } do
let(:milestone?) { false }
before do
# TODO: remove feature flag once the implementation is complete
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(true)
allow(schema)
.to receive(:milestone?)
.and_return(false)
.and_return(milestone?)
end
it_behaves_like 'has basic schema properties' do
@ -313,24 +311,14 @@ describe ::API::V3::WorkPackages::Schema::WorkPackageSchemaRepresenter do
end
context 'when the work package is a milestone' do
before do
allow(schema)
.to receive(:milestone?)
.and_return(true)
end
let(:milestone?) { true }
it 'has no duration attribute' do
expect(subject).not_to have_json_path('duration')
end
end
context 'when the feature flag is off' do
before do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(false)
end
context 'when the feature flag is off', with_flag: { work_packages_duration_field_active: false } do
it 'has no duration attribute' do
expect(subject).not_to have_json_path('duration')
end
@ -348,6 +336,22 @@ describe ::API::V3::WorkPackages::Schema::WorkPackageSchemaRepresenter do
end
end
describe 'ignoreNonWorkingDays', with_flag: { work_packages_duration_field_active: true } do
it_behaves_like 'has basic schema properties' do
let(:path) { 'ignoreNonWorkingDays' }
let(:type) { 'Boolean' }
let(:name) { I18n.t('activerecord.attributes.work_package.ignore_non_working_days') }
let(:required) { false }
let(:writable) { false }
end
context 'when the feature flag is off', with_flag: { work_packages_duration_field_active: false } do
it 'has no ignoreNonWorkingDays attribute' do
expect(subject).not_to have_json_path('ignoreNonWorkingDays')
end
end
end
describe 'date' do
before do
allow(schema)
@ -1082,13 +1086,18 @@ describe ::API::V3::WorkPackages::Schema::WorkPackageSchemaRepresenter do
end
it 'does not cache the attribute_groups' do
representer.to_json
expect(work_package.type)
.to receive(:attribute_groups)
.and_return([])
call_count = 0
allow(work_package.type)
.to receive(:attribute_groups) do
call_count += 1
[]
end
# Rendering two times, the Type#attribute_groups
# should still be called on the second rendering.
representer.to_json
expect { representer.to_json }
.to change { call_count }
end
end

@ -269,32 +269,6 @@ describe ::API::V3::WorkPackages::WorkPackagePayloadRepresenter do
end
end
end
describe 'duration' do
before do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(true)
end
it 'has no duration attribute' do
# TODO: Remove this, once the duration feature is complete
expect(subject).not_to have_json_path('duration')
end
context 'when duration feature flag disabled' do
before do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(false)
end
it 'has no duration attribute' do
# TODO: Remove this, once the duration feature is complete
expect(subject).not_to have_json_path('duration')
end
end
end
end
describe '_links' do

@ -53,6 +53,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
let(:derived_due_date) { Time.zone.today - 5.days }
let(:budget) { build_stubbed(:budget, project:) }
let(:duration) { nil }
let(:ignore_non_working_days) { true }
let(:work_package) do
build_stubbed(:work_package,
schedule_manually:,
@ -69,6 +70,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
estimated_hours:,
derived_estimated_hours:,
budget:,
ignore_non_working_days:,
status:) do |wp|
allow(wp)
.to receive(:available_custom_fields)
@ -297,16 +299,9 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
end
end
describe 'duration' do
describe 'duration', with_flag: { work_packages_duration_field_active: true } do
let(:duration) { 6 }
before do
# TODO: remove feature flag once the implementation is complete
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(true)
end
it { is_expected.to be_json_eql('P6D'.to_json).at_path('duration') }
context 'no duration' do
@ -325,21 +320,33 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
end
end
context 'when the feature flag is off' do
before do
# TODO: remove feature flag once the implementation is complete
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(false)
end
let(:duration) { 6 }
context 'when the feature flag is off', with_flag: { work_packages_duration_field_active: false } do
it 'has no duration' do
is_expected.to_not have_json_path('duration')
end
end
end
describe 'ignoreNonWorkingDays', with_flag: { work_packages_duration_field_active: true } do
let(:ignore_non_working_days) { true }
context 'with the value being `true`' do
it { is_expected.to be_json_eql(true.to_json).at_path('ignoreNonWorkingDays') }
end
context 'with the value being `false`' do
let(:ignore_non_working_days) { false }
it { is_expected.to be_json_eql(false.to_json).at_path('ignoreNonWorkingDays') }
end
context 'when the feature flag is off', with_flag: { work_packages_duration_field_active: false } do
it 'has no ignoreNonWorkingDays' do
is_expected.to_not have_json_path('ignoreNonWorkingDays')
end
end
end
describe 'createdAt' do
it_behaves_like 'has UTC ISO 8601 date and time' do
let(:date) { work_package.created_at }

@ -55,22 +55,14 @@ describe Queries::WorkPackages::Columns::PropertyColumn, type: :model do
end
end
context 'when duration feature flag disabled' do
context 'when duration feature flag disabled', with_flag: { work_packages_duration_field_active: false } do
it 'column does not exist' do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(false)
expect(described_class.instances.map(&:name)).not_to include :duration
end
end
context 'when duration feature flag enabled' do
context 'when duration feature flag enabled', with_flag: { work_packages_duration_field_active: true } do
it 'column exists' do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(true)
expect(described_class.instances.map(&:name)).to include :duration
end
end

@ -113,25 +113,27 @@ describe ::Type, type: :model do
describe '#work_package_attributes' do
subject { type.work_package_attributes }
before do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(true)
end
context 'for the duration field', with_flag: { work_packages_duration_field_active: true } do
it 'does not return true field' do
expect(subject).not_to have_key("duration")
end
it 'does not return the duration field' do
expect(subject).not_to have_key("duration")
context 'when the feature flag is off', with_flag: { work_packages_duration_field_active: false } do
it 'does not return the field' do
expect(subject).not_to have_key("duration")
end
end
end
context 'when the feature flag is off' do
before do
allow(OpenProject::FeatureDecisions)
.to receive(:work_packages_duration_field_active?)
.and_return(false)
context 'for the ignore_non_working_days field', with_flag: { work_packages_duration_field_active: true } do
it 'does not return duration' do
expect(subject).not_to have_key("ignore_non_working_days")
end
it 'does not return the duration field' do
expect(subject).not_to have_key("duration")
context 'when the feature flag is off', with_flag: { work_packages_duration_field_active: false } do
it 'does not return the duration field' do
expect(subject).not_to have_key("ignore_non_working_days")
end
end
end
end

@ -39,7 +39,7 @@ describe WorkPackage, type: :model do
let(:status) { create(:status) }
let(:priority) { create(:priority) }
let(:work_package) do
WorkPackage.new.tap do |w|
described_class.new.tap do |w|
w.attributes = { project_id: project.id,
type_id: type.id,
author_id: user.id,
@ -62,14 +62,14 @@ describe WorkPackage, type: :model do
context 'no project chosen' do
it 'has no type set if no project was chosen' do
expect(WorkPackage.new.type)
expect(described_class.new.type)
.to be_nil
end
end
context 'project chosen' do
it 'has the provided type if one is provided' do
expect(WorkPackage.new(project:, type: type2).type)
expect(described_class.new(project:, type: type2).type)
.to eql type2
end
end
@ -96,7 +96,7 @@ describe WorkPackage, type: :model do
describe 'minimal' do
let(:work_package_minimal) do
WorkPackage.new.tap do |w|
described_class.new.tap do |w|
w.attributes = { project_id: project.id,
type_id: type.id,
author_id: user.id,
@ -277,7 +277,7 @@ describe WorkPackage, type: :model do
end
context 'work package' do
subject { WorkPackage.find_by(id: work_package.id) }
subject { described_class.find_by(id: work_package.id) }
it { is_expected.to be_nil }
end
@ -450,43 +450,43 @@ describe WorkPackage, type: :model do
end
context 'by type' do
let(:groups) { WorkPackage.by_type(project) }
let(:groups) { described_class.by_type(project) }
it_behaves_like 'group by'
end
context 'by version' do
let(:groups) { WorkPackage.by_version(project) }
let(:groups) { described_class.by_version(project) }
it_behaves_like 'group by'
end
context 'by priority' do
let(:groups) { WorkPackage.by_priority(project) }
let(:groups) { described_class.by_priority(project) }
it_behaves_like 'group by'
end
context 'by category' do
let(:groups) { WorkPackage.by_category(project) }
let(:groups) { described_class.by_category(project) }
it_behaves_like 'group by'
end
context 'by assigned to' do
let(:groups) { WorkPackage.by_assigned_to(project) }
let(:groups) { described_class.by_assigned_to(project) }
it_behaves_like 'group by'
end
context 'by responsible' do
let(:groups) { WorkPackage.by_responsible(project) }
let(:groups) { described_class.by_responsible(project) }
it_behaves_like 'group by'
end
context 'by author' do
let(:groups) { WorkPackage.by_author(project) }
let(:groups) { described_class.by_author(project) }
it_behaves_like 'group by'
end
@ -496,7 +496,7 @@ describe WorkPackage, type: :model do
create(:project,
parent: project)
end
let(:groups) { WorkPackage.by_author(project) }
let(:groups) { described_class.by_author(project) }
let(:work_package_3) do
create(:work_package,
project: project_2)
@ -523,7 +523,7 @@ describe WorkPackage, type: :model do
end
context 'limit' do
subject { WorkPackage.recently_updated.limit(1).first }
subject { described_class.recently_updated.limit(1).first }
it { is_expected.to eq(work_package_2) }
end
@ -540,7 +540,7 @@ describe WorkPackage, type: :model do
project: project_archived)
end
subject { WorkPackage.on_active_project.length }
subject { described_class.on_active_project.length }
context 'one work package in active projects' do
it { is_expected.to eq(1) }
@ -566,7 +566,7 @@ describe WorkPackage, type: :model do
author: user)
end
subject { WorkPackage.with_author(user).length }
subject { described_class.with_author(user).length }
context 'one work package in active projects' do
it { is_expected.to eq(1) }
@ -608,7 +608,7 @@ describe WorkPackage, type: :model do
context 'when having the move_work_packages permission' do
it 'returns the project' do
expect(WorkPackage.allowed_target_projects_on_move(user))
expect(described_class.allowed_target_projects_on_move(user))
.to match_array [project]
end
end
@ -617,7 +617,7 @@ describe WorkPackage, type: :model do
let(:role) { create(:role, permissions: []) }
it 'does not return the project' do
expect(WorkPackage.allowed_target_projects_on_move(user))
expect(described_class.allowed_target_projects_on_move(user))
.to be_empty
end
end
@ -632,7 +632,7 @@ describe WorkPackage, type: :model do
context 'when having the add_work_packages permission' do
it 'returns the project' do
expect(WorkPackage.allowed_target_projects_on_create(user))
expect(described_class.allowed_target_projects_on_create(user))
.to match_array [project]
end
end
@ -641,7 +641,7 @@ describe WorkPackage, type: :model do
let(:role) { create(:role, permissions: []) }
it 'does not return the project' do
expect(WorkPackage.allowed_target_projects_on_create(user))
expect(described_class.allowed_target_projects_on_create(user))
.to be_empty
end
end
@ -673,21 +673,30 @@ describe WorkPackage, type: :model do
end
describe 'null' do
subject { WorkPackage.changed_since(nil) }
subject { described_class.changed_since(nil) }
it { expect(subject).to match_array([work_package]) }
end
describe 'now' do
subject { WorkPackage.changed_since(DateTime.now) }
subject { described_class.changed_since(DateTime.now) }
it { expect(subject).to be_empty }
end
describe 'work package update' do
subject { WorkPackage.changed_since(work_package.reload.updated_at) }
subject { described_class.changed_since(work_package.reload.updated_at) }
it { expect(subject).to match_array([work_package]) }
end
end
describe '#ignore_non_working_days' do
context 'for a new record' do
it 'is true' do
expect(described_class.new.ignore_non_working_days)
.to be true
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

@ -29,7 +29,7 @@
require 'spec_helper'
require 'rack/test'
describe 'OAuthClient callback endpoint', :enable_storages, type: :request do
describe 'OAuthClient callback endpoint', with_flag: { storages_module_active: true }, type: :request do
include Rack::Test::Methods
include API::V3::Utilities::PathHelper

@ -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

@ -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 }
# 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
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' } }
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

@ -0,0 +1,47 @@
# 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.
module WithFlagMixin
module_function
def with_flags(flags)
flags.each do |k, value|
name = k.end_with?('?') ? k : "#{k}?"
raise "#{k} is not a valid flag" unless OpenProject::FeatureDecisions.respond_to?(name)
allow(OpenProject::FeatureDecisions).to receive(name).and_return value
end
end
end
RSpec.configure do |config|
config.include WithFlagMixin
config.before :example, :with_flag do |example|
with_flags(example.metadata[:with_flag])
end
end
Loading…
Cancel
Save