From 88c037cbaf1a27786b62eee1a9a003e0f97ea7e6 Mon Sep 17 00:00:00 2001 From: Marcello Rocha Date: Fri, 20 Dec 2024 17:10:26 +0100 Subject: [PATCH] Changes the Update / Create work package contract. --- app/models/type.rb | 43 +++++---- app/models/types/pattern.rb | 7 +- app/models/types/pattern_mapper.rb | 53 +++++++++-- .../collection.rb} | 34 ++++--- .../collection_contract.rb} | 12 +-- .../collection_type.rb} | 32 +++---- app/models/types/patterns/token.rb | 48 +++++++++- app/services/work_packages/create_service.rb | 38 ++++---- .../work_packages/set_attributes_service.rb | 18 +++- app/services/work_packages/update_service.rb | 11 +++ spec/factories/type_factory.rb | 3 + spec/models/type_spec.rb | 4 +- .../set_attributes_service_spec.rb | 89 ++++++++++++------- 13 files changed, 273 insertions(+), 119 deletions(-) rename app/models/types/{pattern_collection.rb => patterns/collection.rb} (66%) rename app/models/types/{pattern_collection_contract.rb => patterns/collection_contract.rb} (84%) rename app/models/types/{pattern_collection_type.rb => patterns/collection_type.rb} (73%) diff --git a/app/models/type.rb b/app/models/type.rb index 0117f8aaa6b8..9cd0ee07ef88 100644 --- a/app/models/type.rb +++ b/app/models/type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -34,10 +36,12 @@ class Type < ApplicationRecord include ::Scopes::Scoped - attribute :patterns, Types::PatternCollectionType.new + attribute :patterns, Types::Patterns::CollectionType.new before_destroy :check_integrity + belongs_to :color, optional: true, class_name: "Color" + has_many :work_packages has_many :workflows, dependent: :delete_all do def copy_from_type(source_type) @@ -52,12 +56,9 @@ def copy_from_type(source_type) join_table: "#{table_name_prefix}custom_fields_types#{table_name_suffix}", association_foreign_key: "custom_field_id" - belongs_to :color, optional: true, class_name: "Color" - acts_as_list validates :name, presence: true, uniqueness: { case_sensitive: false }, length: { maximum: 255 } - validates :is_default, :is_milestone, inclusion: { in: [true, false] } scopes :milestone @@ -65,8 +66,9 @@ def copy_from_type(source_type) default_scope { order("position ASC") } scope :without_standard, -> { where(is_standard: false).order(:position) } + scope :default, -> { where(is_default: true) } - def to_s; name end + delegate :to_s, to: :name def <=>(other) name <=> other.name @@ -81,26 +83,20 @@ def self.statuses(types) end def self.standard_type - ::Type.where(is_standard: true).first - end - - def self.default - ::Type.where(is_default: true) + where(is_standard: true).first end def self.enabled_in(project) - ::Type.includes(:projects).where(projects: { id: project }) + includes(:projects).where(projects: { id: project }) end def statuses(include_default: false) if new_record? Status.none elsif include_default - ::Type - .statuses([id]) - .or(Status.where_default) + self.class.statuses([id]).or(Status.where_default) else - ::Type.statuses([id]) + self.class.statuses([id]) end end @@ -108,9 +104,24 @@ def enabled_in?(object) object.types.include?(self) end + def replacement_patterns_defined? + return false if patterns.blank? + + patterns.all_enabled.any? + end + + def enabled_patterns + return {} if patterns.blank? + + patterns.all_enabled + end + private def check_integrity - raise "Can't delete type" if WorkPackage.where(type_id: id).any? + throw :abort if is_standard? + throw :abort if WorkPackage.exists?(type_id: id) + + true end end diff --git a/app/models/types/pattern.rb b/app/models/types/pattern.rb index 62dcbd2b2c18..8442fcac254f 100644 --- a/app/models/types/pattern.rb +++ b/app/models/types/pattern.rb @@ -30,9 +30,10 @@ module Types Pattern = Data.define(:blueprint, :enabled) do - def call(object) - # calculate string using object - blueprint.to_s + object.to_s + def enabled? = !!enabled + + def resolve(work_package) + PatternMapper.new(self).resolve(work_package) end def to_h diff --git a/app/models/types/pattern_mapper.rb b/app/models/types/pattern_mapper.rb index 4db11198104c..b96399e54e88 100644 --- a/app/models/types/pattern_mapper.rb +++ b/app/models/types/pattern_mapper.rb @@ -1,40 +1,77 @@ # frozen_string_literal: true #-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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 Types class PatternMapper - TOKEN_REGEX = /{{[A-z_]+}}/ + TOKEN_REGEX = /{{[0-9A-z_]+}}/ MAPPING = { - assignee: ->(wp) { wp.assigned_to.name }, + type: ->(wp) { wp.type.name }, + assignee: ->(wp) { wp.assigned_to&.name }, created: ->(wp) { wp.created_at }, - author: ->(wp) { wp.author.name } + author: ->(wp) { wp.author.name }, + parent_id: ->(wp) { wp.parent&.id }, + project_name: ->(wp) { wp.project.name } }.freeze private_constant :MAPPING def initialize(pattern) @pattern = pattern - @tokens = pattern.scan(TOKEN_REGEX).map { |token| Patterns::Token.new(token) } + @tokens = pattern.scan(TOKEN_REGEX).map { |token| Patterns::Token.build(token) } end def valid?(work_package) - @tokens.each { |token| fn(token.key).call(work_package) } + @tokens.each { |token| get_value(work_package, token) } rescue NoMethodError false end def resolve(work_package) @tokens.inject(@pattern) do |pattern, token| - value = fn(token.key).call(work_package) - pattern.gsub(token.pattern, stringify(value)) + pattern.gsub(token.pattern, get_value(work_package, token)) end end private + def get_value(work_package, token) + raw_value = if token.custom_field? && token.context != work_package.context + fn(key).call(work_package.public_send(token.context)) + else + fn(token.key).call(work_package) + end + + stringify(raw_value) + end + def fn(key) MAPPING.fetch(key) { ->(wp) { wp.public_send(key.to_sym) } } end @@ -43,6 +80,8 @@ def stringify(value) case value when Date, Time, DateTime value.strftime("%Y-%m-%d") + when NilClass + "PITY DA FOOL!" else value.to_s end diff --git a/app/models/types/pattern_collection.rb b/app/models/types/patterns/collection.rb similarity index 66% rename from app/models/types/pattern_collection.rb rename to app/models/types/patterns/collection.rb index 52a6dda2e459..990a7b9413c6 100644 --- a/app/models/types/pattern_collection.rb +++ b/app/models/types/patterns/collection.rb @@ -29,25 +29,31 @@ #++ module Types - PatternCollection = Data.define(:patterns) do - private_class_method :new + module Patterns + Collection = Data.define(:patterns) do + private_class_method :new - def self.build(patterns:, contract: PatternCollectionContract.new) - contract.call(patterns).to_monad.fmap { |success| new(success.to_h) } - end + def self.build(patterns:, contract: CollectionContract.new) + contract.call(patterns).to_monad.fmap { |success| new(success.to_h) } + end - def initialize(patterns:) - transformed = patterns.transform_values { Pattern.new(**_1) }.freeze + def initialize(patterns:) + transformed = patterns.transform_values { Pattern.new(**_1) }.freeze - super(patterns: transformed) - end + super(patterns: transformed) + end - def [](value) - patterns.fetch(value) - end + def all_enabled + patterns.select { |_, pattern| pattern.enabled? } + end + + def [](value) + patterns.fetch(value) + end - def to_h - patterns.stringify_keys.transform_values(&:to_h) + def to_h + patterns.stringify_keys.transform_values(&:to_h) + end end end end diff --git a/app/models/types/pattern_collection_contract.rb b/app/models/types/patterns/collection_contract.rb similarity index 84% rename from app/models/types/pattern_collection_contract.rb rename to app/models/types/patterns/collection_contract.rb index 2a8e7c16a1e1..02f79b288e4e 100644 --- a/app/models/types/pattern_collection_contract.rb +++ b/app/models/types/patterns/collection_contract.rb @@ -29,11 +29,13 @@ #++ module Types - class PatternCollectionContract < Dry::Validation::Contract - params do - required(:subject).hash do - required(:blueprint).filled(:string) - required(:enabled).filled(:bool) + module Patterns + class CollectionContract < Dry::Validation::Contract + params do + required(:subject).hash do + required(:blueprint).filled(:string) + required(:enabled).filled(:bool) + end end end end diff --git a/app/models/types/pattern_collection_type.rb b/app/models/types/patterns/collection_type.rb similarity index 73% rename from app/models/types/pattern_collection_type.rb rename to app/models/types/patterns/collection_type.rb index 9758ee50c339..932cf17ec335 100644 --- a/app/models/types/pattern_collection_type.rb +++ b/app/models/types/patterns/collection_type.rb @@ -29,26 +29,28 @@ #++ module Types - class PatternCollectionType < ActiveModel::Type::Value - def assert_valid_value(value) - cast(value) - end + module Patterns + class CollectionType < ActiveModel::Type::Value + def assert_valid_value(value) + cast(value) + end - def cast(value) - PatternCollection.build(patterns: value).value_or { nil } - end + def cast(value) + Collection.build(patterns: value).value_or { nil } + end - def serialize(pattern) - return super if pattern.nil? + def serialize(pattern) + return super if pattern.nil? - YAML.dump(pattern.to_h) - end + YAML.dump(pattern.to_h) + end - def deserialize(value) - return if value.blank? + def deserialize(value) + return if value.blank? - data = YAML.safe_load(value) - cast(data) + data = YAML.safe_load(value) + cast(data) + end end end end diff --git a/app/models/types/patterns/token.rb b/app/models/types/patterns/token.rb index 1ae8ff88b4f5..ee525794cc55 100644 --- a/app/models/types/patterns/token.rb +++ b/app/models/types/patterns/token.rb @@ -1,12 +1,56 @@ # frozen_string_literal: true #-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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 Types module Patterns - Token = Data.define(:pattern) do - def key = pattern.tr("{}", "").to_sym + Token = Data.define(:pattern, :key) do + private_class_method :new + + def self.build(pattern) + new(pattern, pattern.tr("{}", "").to_sym) + end + + def custom_field? = key.include?("custom_field") + + def custom_field_id + return nil unless custom_field? + + Integer(key.to_s.gsub(/\D+/, "")) + end + + def custom_field_context + context = key.to_s.gsub(/_?custom_field_\d+/, "") + return :work_package if context.blank? + + context.to_sym + end end end end diff --git a/app/services/work_packages/create_service.rb b/app/services/work_packages/create_service.rb index 600c0f5d0eb2..405b863a72fb 100644 --- a/app/services/work_packages/create_service.rb +++ b/app/services/work_packages/create_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -30,17 +32,14 @@ class WorkPackages::CreateService < BaseServices::BaseCallable include ::WorkPackages::Shared::UpdateAncestors include ::Shared::ServiceContext - attr_accessor :user, - :contract_class + attr_reader :user, :contract_class def initialize(user:, contract_class: WorkPackages::CreateContract) - self.user = user - self.contract_class = contract_class + @user = user + @contract_class = contract_class end - def perform(work_package: WorkPackage.new, - send_notifications: nil, - **attributes) + def perform(work_package: WorkPackage.new, send_notifications: nil, **attributes) in_user_context(send_notifications:) do create(attributes, work_package) end @@ -55,6 +54,8 @@ def create(attributes, work_package) if result.success work_package.attachments = work_package.attachments_replacements if work_package.attachments_replacements work_package.save + set_templated_subject(work_package) + work_package.save else false end @@ -74,18 +75,19 @@ def create(attributes, work_package) result end - def set_attributes(attributes, wp) - attributes_service_class - .new(user:, - model: wp, - contract_class:) - .call(attributes) + def set_templated_subject(work_package) + return unless work_package.type&.replacement_patterns_defined? + return unless work_package.type.enabled_patterns[:subject] + + work_package.subject = work_package.type.enabled_patterns[:subject].resolve(work_package) + end + + def set_attributes(attributes, work_package) + attributes_service_class.new(user:, model: work_package, contract_class:).call(attributes) end def reschedule_related(work_package) - result = WorkPackages::SetScheduleService - .new(user:, work_package:) - .call + result = WorkPackages::SetScheduleService.new(user:, work_package:).call result.self_and_dependent.each do |r| unless r.result.save @@ -100,9 +102,7 @@ def reschedule_related(work_package) def set_user_as_watcher(work_package) # We don't care if it fails here. If it does # the user simply does not become watcher - Services::CreateWatcher - .new(work_package, user) - .run(send_notifications: false) + Services::CreateWatcher.new(work_package, user).run(send_notifications: false) end def attributes_service_class diff --git a/app/services/work_packages/set_attributes_service.rb b/app/services/work_packages/set_attributes_service.rb index cd1118d965af..264cdeb57b1b 100644 --- a/app/services/work_packages/set_attributes_service.rb +++ b/app/services/work_packages/set_attributes_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -43,6 +45,15 @@ def set_attributes(attributes) end set_custom_attributes(attributes) + mark_templated_subject + end + + def mark_templated_subject + return true unless work_package.type&.replacement_patterns_defined? + + if work_package.type.patterns.all_enabled[:subject] + work_package.subject = "Templated by #{work_package.type.name}" + end end def set_static_attributes(attributes) @@ -298,15 +309,14 @@ def derive_progress_values_class def set_version_to_nil if work_package.version && - work_package.project && - work_package.project.shared_versions.exclude?(work_package.version) + work_package.project&.shared_versions&.exclude?(work_package.version) work_package.version = nil end end def set_parent_to_nil if !Setting.cross_project_work_package_relations? && - !work_package.parent_changed? + !work_package.parent_changed? work_package.parent = nil end @@ -368,7 +378,7 @@ def new_start_date def new_start_date_from_parent return unless work_package.parent_id_changed? && - work_package.parent + work_package.parent work_package.parent.soonest_start end diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb index 4788e7374239..ee45cd51626b 100644 --- a/app/services/work_packages/update_service.rb +++ b/app/services/work_packages/update_service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -39,7 +41,16 @@ def initialize(user:, model:, contract_class: nil, contract_options: {}, cause_o private + def set_templated_attributes + return unless model.type.replacement_patterns_defined? + + model.type.patterns.all_enabled.each do |key, pattern| + model.public_send(:"#{key}=", pattern.resolve(model)) + end + end + def after_perform(service_call) + set_templated_attributes update_related_work_packages(service_call) cleanup(service_call.result) diff --git a/spec/factories/type_factory.rb b/spec/factories/type_factory.rb index 180fe4db92ae..27c3beb06ce5 100644 --- a/spec/factories/type_factory.rb +++ b/spec/factories/type_factory.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -31,6 +33,7 @@ sequence(:position) name { |a| "Type No. #{a.position}" } description { nil } + patterns { nil } created_at { Time.zone.now } updated_at { Time.zone.now } diff --git a/spec/models/type_spec.rb b/spec/models/type_spec.rb index 7f4073acc72c..7a1e0d25dd67 100644 --- a/spec/models/type_spec.rb +++ b/spec/models/type_spec.rb @@ -140,7 +140,7 @@ subject: { blueprint: "{{work_package:custom_field_123}} - {{project:custom_field_321}}", enabled: true } }) - expect(type.patterns).to be_a(Types::PatternCollection) + expect(type.patterns).to be_a(Types::Patterns::Collection) expect(type.patterns[:subject]) .to eq(Types::Pattern.new("{{work_package:custom_field_123}} - {{project:custom_field_321}}", true)) end @@ -161,7 +161,7 @@ it "converts the incoming hash into a PatternCollection" do type.patterns = { subject: { blueprint: "some_string", enabled: false } } - expect(type.patterns).to be_a(Types::PatternCollection) + expect(type.patterns).to be_a(Types::Patterns::Collection) expect(type.patterns[:subject]).to be_a(Types::Pattern) expect { type.save! }.not_to raise_error diff --git a/spec/services/work_packages/set_attributes_service_spec.rb b/spec/services/work_packages/set_attributes_service_spec.rb index 0915781acf4e..bfdbc1b92735 100644 --- a/spec/services/work_packages/set_attributes_service_spec.rb +++ b/spec/services/work_packages/set_attributes_service_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -43,6 +45,7 @@ p end + let(:work_package) do wp = build_stubbed(:work_package, project:, status: status_0_pct_complete) wp.type = initial_type @@ -50,9 +53,7 @@ wp end - let(:new_work_package) do - WorkPackage.new - end + let(:new_work_package) { WorkPackage.new } let(:initial_type) { build_stubbed(:type) } let(:milestone_type) { build_stubbed(:type_milestone) } let(:statuses) { [] } @@ -78,24 +79,25 @@ end shared_examples_for "service call" do |description: nil| - subject do - allow(work_package) - .to receive(:save) + subject(:service_result) { instance.call(call_attributes) } - instance.call(call_attributes) - end + before { allow(work_package).to receive(:save) } it description || "sets the value" do all_expected_attributes = {} all_expected_attributes.merge!(expected_attributes) if defined?(expected_attributes) + if defined?(expected_kept_attributes) kept = work_package.attributes.slice(*expected_kept_attributes) + if kept.size != expected_kept_attributes.size raise ArgumentError, "expected_kept_attributes contains attributes that are not present in the work_package: " \ "#{expected_kept_attributes - kept.keys} not present in #{work_package.attributes}" end + all_expected_attributes.merge!(kept) end + next if all_expected_attributes.blank? subject @@ -354,9 +356,7 @@ let(:new_statuses) { [other_status, default_status] } before do - allow(Status) - .to receive(:default) - .and_return(default_status) + allow(Status).to receive(:default).and_return(default_status) end context "with no value set before for a new work package" do @@ -364,9 +364,7 @@ let(:expected_attributes) { { status: default_status } } let(:work_package) { new_work_package } - before do - work_package.status = nil - end + before { work_package.status = nil } it_behaves_like "service call" end @@ -447,17 +445,15 @@ it_behaves_like "service call" do it "sets the service's author" do - subject + instance.call(call_attributes) - expect(work_package.author) - .to eql user + expect(work_package.author).to eql user end it "notes the author to be system changed" do subject - expect(work_package.changed_by_system["author_id"]) - .to eql [nil, user.id] + expect(work_package.changed_by_system["author_id"]).to eql [nil, user.id] end end end @@ -565,7 +561,7 @@ before do allow(parent) .to receive(:soonest_start) - .and_return(parent_start_date + 3.days) + .and_return(parent_start_date + 3.days) end it_behaves_like "service call" do @@ -1475,10 +1471,10 @@ allow(IssuePriority) .to receive(:active) - .and_return(scope) + .and_return(scope) allow(scope) .to receive(:default) - .and_return(default_priority) + .and_return(default_priority) end context "with no value set before for a new work package" do @@ -1639,22 +1635,22 @@ instance_double(ActiveRecord::Relation).tap do |categories_stub| allow(new_project) .to receive(:categories) - .and_return(categories_stub) + .and_return(categories_stub) end end before do without_partial_double_verification do allow(new_project_categories) - .to receive(:find_by) - .with(name: category.name) - .and_return nil + .to receive(:find_by) + .with(name: category.name) + .and_return nil allow(new_project) .to receive_messages(shared_versions: new_versions, types: new_types) allow(new_types) .to receive(:order) - .with(:position) - .and_return(new_types) + .with(:position) + .and_return(new_types) end end @@ -1703,8 +1699,8 @@ before do allow(new_project_categories) .to receive(:find_by) - .with(name: category.name) - .and_return new_category + .with(name: category.name) + .and_return new_category end it "uses the equally named category" do @@ -1859,7 +1855,7 @@ allow(wp) .to receive(:soonest_start) - .and_return(soonest_start) + .and_return(soonest_start) wp end @@ -1940,7 +1936,7 @@ before do allow(work_package) .to receive(:children) - .and_return([child]) + .and_return([child]) end context "when the child`s start date is after soonest_start" do @@ -1968,4 +1964,33 @@ end end end + + context "when the type defines a pattern for an attribute" do + let(:type) { build_stubbed(:type, patterns: { subject: { blueprint: "{{type}} {{project_name}}", enabled: true } }) } + let(:work_package) { WorkPackage.new(type:) } + + it "assigns a to be updated value to the field" do + instance.call({}) + + expect(work_package.subject).to eq("Templated by #{type.name}") + end + + it "overrides even a passed subject" do + instance.call(subject: "I will be overwritten") + + expect(work_package.subject).to eq("Templated by #{type.name}") + end + + context "when the pattern is disabled" do + let(:type) do + build_stubbed(:type, patterns: { subject: { blueprint: "{{type}} {{project_name}}", enabled: false } }) + end + + it "does not overwrite the attribute" do + instance.call(subject: "I will be kept") + + expect(work_package.subject).to eq("I will be kept") + end + end + end end