From bb5f62da76f1e691ca1546e5e3f08d52b95579f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 12 May 2026 10:22:33 +0200 Subject: [PATCH] Update automation actions to be separate STI table --- app/contracts/automations/cu_contract.rb | 5 +- app/models/automation.rb | 23 ++-- app/models/automations/actions/base.rb | 35 +++-- .../automations/actions/custom_field.rb | 103 ++++++--------- .../actions/custom_field/for_associated.rb | 5 + .../actions/custom_field/for_boolean.rb | 6 + .../actions/custom_field/for_date.rb | 6 + .../actions/custom_field/for_float.rb | 6 + .../actions/custom_field/for_integer.rb | 6 + .../actions/custom_field/for_link.rb | 6 + .../actions/custom_field/for_string.rb | 6 + .../actions/custom_field/for_text.rb | 6 + .../actions/custom_field/for_user.rb | 5 + app/models/automations/actions/done_ratio.rb | 2 +- app/models/automations/actions/serializer.rb | 57 --------- .../automations/actions/strategies/date.rb | 2 +- .../actions/strategies/me_associated.rb | 4 +- app/services/automations/base_service.rb | 53 ++++---- ...0_convert_custom_actions_to_automations.rb | 120 ++++++++++++++++++ .../contracts/automations/cu_contract_spec.rb | 9 +- .../automations/automations_spec.rb | 2 +- .../work_package_representer_spec.rb | 4 +- spec/models/automation_spec.rb | 11 +- .../automations/actions/custom_field_spec.rb | 35 ++--- .../strategies/user_custom_field_spec.rb | 4 +- .../models/automations/shared_expectations.rb | 10 +- .../custom_actions/custom_actions_api_spec.rb | 6 +- .../automations/update_service_spec.rb | 9 +- 28 files changed, 309 insertions(+), 237 deletions(-) create mode 100644 app/models/automations/actions/custom_field/for_associated.rb create mode 100644 app/models/automations/actions/custom_field/for_boolean.rb create mode 100644 app/models/automations/actions/custom_field/for_date.rb create mode 100644 app/models/automations/actions/custom_field/for_float.rb create mode 100644 app/models/automations/actions/custom_field/for_integer.rb create mode 100644 app/models/automations/actions/custom_field/for_link.rb create mode 100644 app/models/automations/actions/custom_field/for_string.rb create mode 100644 app/models/automations/actions/custom_field/for_text.rb create mode 100644 app/models/automations/actions/custom_field/for_user.rb delete mode 100644 app/models/automations/actions/serializer.rb diff --git a/app/contracts/automations/cu_contract.rb b/app/contracts/automations/cu_contract.rb index 258a3f63bd8..eecc3cafa20 100644 --- a/app/contracts/automations/cu_contract.rb +++ b/app/contracts/automations/cu_contract.rb @@ -12,8 +12,9 @@ module Automations attribute :description attribute :actions do - errors.add(:actions, :empty) if model.actions.empty? - model.actions.each { |action| action.validate(errors) } + live_actions = model.actions.reject(&:marked_for_destruction?) + errors.add(:actions, :empty) if live_actions.empty? + live_actions.each { |action| action.validate(errors) } end attribute :conditions do diff --git a/app/models/automation.rb b/app/models/automation.rb index 4458809a59b..c5791fd62f4 100644 --- a/app/models/automation.rb +++ b/app/models/automation.rb @@ -5,7 +5,6 @@ class Automation < ApplicationRecord validate :must_have_at_least_one_trigger validate :must_not_have_more_than_one_manual_trigger - serialize :actions, coder: Automations::Actions::Serializer before_validation :ensure_manual_trigger has_and_belongs_to_many :status_conditions, class_name: "Status", join_table: :automations_statuses @@ -20,6 +19,13 @@ class Automation < ApplicationRecord inverse_of: :automation accepts_nested_attributes_for :triggers + has_many :actions, + -> { order(:position, :id) }, + class_name: "Automations::Actions::Base", + dependent: :destroy, + inverse_of: :automation + accepts_nested_attributes_for :actions, allow_destroy: true + after_save :persist_conditions attribute :conditions @@ -31,22 +37,11 @@ class Automation < ApplicationRecord joins(:triggers).where(automation_triggers: { type: "Automations::Triggers::Manual" }).distinct } - def initialize(*args) - ret = super - self.actions ||= [] - ret - end - def reload(*args) @conditions = nil super end - def actions=(values) - actions_will_change! - super - end - def self.order_by_name order(:name) end @@ -60,7 +55,7 @@ class Automation < ApplicationRecord end def available_actions - ::Automations::Register.actions.map(&:all).flatten + ::Automations::Register.actions.flat_map(&:templates) end def all_conditions @@ -102,7 +97,7 @@ class Automation < ApplicationRecord availables.map do |available| existing = actual.detect { |a| a.key == available.key } - existing || available.new + existing || (available.is_a?(Class) ? available.new : available) end end diff --git a/app/models/automations/actions/base.rb b/app/models/automations/actions/base.rb index aae8e4bb925..80277a80807 100644 --- a/app/models/automations/actions/base.rb +++ b/app/models/automations/actions/base.rb @@ -28,17 +28,24 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Automations::Actions::Base - attr_reader :values +class Automations::Actions::Base < ApplicationRecord + self.table_name = "automation_actions" DEFAULT_PRIORITY = 100 - def initialize(values = []) - self.values = values + belongs_to :automation, inverse_of: :actions + acts_as_list scope: :automation + + store_attribute :options, :values + + after_initialize :coerce_persisted_values + + def values + Array(super) end - def values=(values) - @values = Array(values) + def write_raw_values(new_values) + self.options = options.is_a?(Hash) ? options.merge("values" => Array(new_values)) : { "values" => Array(new_values) } end def allowed_values @@ -67,14 +74,8 @@ class Automations::Actions::Base raise SubclassResponsibilityError end - def self.all - [self] - end - - def self.for(key) - if key == self.key - self - end + def self.templates + [new] end delegate :key, to: :class @@ -98,6 +99,12 @@ class Automations::Actions::Base private + def coerce_persisted_values + return if new_record? || values.empty? + + self.values = values + end + def validate_value_required(errors) if required? && values.empty? errors.add :actions, diff --git a/app/models/automations/actions/custom_field.rb b/app/models/automations/actions/custom_field.rb index 4b5dca1692b..60d95387ef5 100644 --- a/app/models/automations/actions/custom_field.rb +++ b/app/models/automations/actions/custom_field.rb @@ -29,80 +29,50 @@ #++ class Automations::Actions::CustomField < Automations::Actions::Base - class << self - def key - custom_field.attribute_name.to_sym + store_attribute :options, :custom_field_id, :integer + + FORMAT_TO_SUBCLASS = { + "string" => "Automations::Actions::CustomField::ForString", + "text" => "Automations::Actions::CustomField::ForText", + "link" => "Automations::Actions::CustomField::ForLink", + "int" => "Automations::Actions::CustomField::ForInteger", + "float" => "Automations::Actions::CustomField::ForFloat", + "date" => "Automations::Actions::CustomField::ForDate", + "bool" => "Automations::Actions::CustomField::ForBoolean", + "user" => "Automations::Actions::CustomField::ForUser", + "list" => "Automations::Actions::CustomField::ForAssociated", + "version" => "Automations::Actions::CustomField::ForAssociated" + }.freeze + + def self.templates + WorkPackageCustomField.usable_as_automation.filter_map do |cf| + subclass = subclass_for(cf) + next unless subclass + + template = subclass.new(custom_field_id: cf.id) + template.instance_variable_set(:@custom_field, cf) + template end + end - def custom_field - raise SubclassResponsibilityError - end - - def all - WorkPackageCustomField - .usable_as_automation - .map do |cf| - create_subclass(cf) - end - end - - def for(key) - match_result = key.match /custom_field_(\d+)/ - - if match_result && (cf = WorkPackageCustomField.find_by(id: match_result[1])) - create_subclass(cf) - end - end - - private - - def create_subclass(custom_field) - klass = Class.new(Automations::Actions::CustomField) - klass.define_singleton_method(:custom_field) do - custom_field - end - - klass.include(strategy(custom_field)) - klass - end - - def strategy(custom_field) - case custom_field.field_format - when "string" - Automations::Actions::Strategies::String - when "text" - Automations::Actions::Strategies::Text - when "link" - Automations::Actions::Strategies::Link - when "int" - Automations::Actions::Strategies::Integer - when "float" - Automations::Actions::Strategies::Float - when "date" - Automations::Actions::Strategies::Date - when "bool" - Automations::Actions::Strategies::Boolean - when "user" - Automations::Actions::Strategies::UserCustomField - when "list", "version" - Automations::Actions::Strategies::AssociatedCustomField - end - end + def self.subclass_for(custom_field) + name = FORMAT_TO_SUBCLASS[custom_field.field_format] + name&.constantize end def custom_field - self.class.custom_field + return nil if custom_field_id.blank? + + @custom_field ||= WorkPackageCustomField.find_by(id: custom_field_id) + end + + def key + cf = custom_field + cf ? cf.attribute_name.to_sym : :inexistent_custom_field end def human_name - custom_field.name - end - - def apply(work_package) - if work_package.respond_to?(custom_field.attribute_setter) - set_custom_field_value(work_package) - validate_custom_field(work_package) - end + custom_field&.name || super end private @@ -112,7 +82,6 @@ class Automations::Actions::CustomField < Automations::Actions::Base end def validate_custom_field(work_package) - # Validate the custom field the custom action is changing. work_package.custom_values_to_validate += Array(work_package.custom_value_for(custom_field)) end end diff --git a/app/models/automations/actions/custom_field/for_associated.rb b/app/models/automations/actions/custom_field/for_associated.rb new file mode 100644 index 00000000000..ac24f90d2b9 --- /dev/null +++ b/app/models/automations/actions/custom_field/for_associated.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForAssociated < Automations::Actions::CustomField + include Automations::Actions::Strategies::AssociatedCustomField +end diff --git a/app/models/automations/actions/custom_field/for_boolean.rb b/app/models/automations/actions/custom_field/for_boolean.rb new file mode 100644 index 00000000000..f6eebfecc9e --- /dev/null +++ b/app/models/automations/actions/custom_field/for_boolean.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForBoolean < Automations::Actions::CustomField + include Automations::Actions::Strategies::Boolean + include Automations::Actions::Strategies::CustomField +end diff --git a/app/models/automations/actions/custom_field/for_date.rb b/app/models/automations/actions/custom_field/for_date.rb new file mode 100644 index 00000000000..bdcdf1bfa2e --- /dev/null +++ b/app/models/automations/actions/custom_field/for_date.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForDate < Automations::Actions::CustomField + include Automations::Actions::Strategies::CustomField + include Automations::Actions::Strategies::Date +end diff --git a/app/models/automations/actions/custom_field/for_float.rb b/app/models/automations/actions/custom_field/for_float.rb new file mode 100644 index 00000000000..fd793dfb4b9 --- /dev/null +++ b/app/models/automations/actions/custom_field/for_float.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForFloat < Automations::Actions::CustomField + include Automations::Actions::Strategies::Float + include Automations::Actions::Strategies::CustomField +end diff --git a/app/models/automations/actions/custom_field/for_integer.rb b/app/models/automations/actions/custom_field/for_integer.rb new file mode 100644 index 00000000000..e280efbc572 --- /dev/null +++ b/app/models/automations/actions/custom_field/for_integer.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForInteger < Automations::Actions::CustomField + include Automations::Actions::Strategies::Integer + include Automations::Actions::Strategies::CustomField +end diff --git a/app/models/automations/actions/custom_field/for_link.rb b/app/models/automations/actions/custom_field/for_link.rb new file mode 100644 index 00000000000..7af4e73c2ee --- /dev/null +++ b/app/models/automations/actions/custom_field/for_link.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForLink < Automations::Actions::CustomField + include Automations::Actions::Strategies::Link + include Automations::Actions::Strategies::CustomField +end diff --git a/app/models/automations/actions/custom_field/for_string.rb b/app/models/automations/actions/custom_field/for_string.rb new file mode 100644 index 00000000000..6b4ffd316c0 --- /dev/null +++ b/app/models/automations/actions/custom_field/for_string.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForString < Automations::Actions::CustomField + include Automations::Actions::Strategies::String + include Automations::Actions::Strategies::CustomField +end diff --git a/app/models/automations/actions/custom_field/for_text.rb b/app/models/automations/actions/custom_field/for_text.rb new file mode 100644 index 00000000000..1ad39881bcf --- /dev/null +++ b/app/models/automations/actions/custom_field/for_text.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForText < Automations::Actions::CustomField + include Automations::Actions::Strategies::Text + include Automations::Actions::Strategies::CustomField +end diff --git a/app/models/automations/actions/custom_field/for_user.rb b/app/models/automations/actions/custom_field/for_user.rb new file mode 100644 index 00000000000..e393a3f9141 --- /dev/null +++ b/app/models/automations/actions/custom_field/for_user.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Automations::Actions::CustomField::ForUser < Automations::Actions::CustomField + include Automations::Actions::Strategies::UserCustomField +end diff --git a/app/models/automations/actions/done_ratio.rb b/app/models/automations/actions/done_ratio.rb index d51ecec36b9..36e5fcfb0f2 100644 --- a/app/models/automations/actions/done_ratio.rb +++ b/app/models/automations/actions/done_ratio.rb @@ -47,7 +47,7 @@ class Automations::Actions::DoneRatio < Automations::Actions::Base 100 end - def self.all + def self.templates if WorkPackage.work_based_mode? super else diff --git a/app/models/automations/actions/serializer.rb b/app/models/automations/actions/serializer.rb deleted file mode 100644 index 1905419e896..00000000000 --- a/app/models/automations/actions/serializer.rb +++ /dev/null @@ -1,57 +0,0 @@ -# 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 Automations::Actions::Serializer - module_function - - def load(value) - return [] unless value - - YAML - .safe_load(value, permitted_classes: [Symbol, Date]) - .filter_map do |key, values| - klass = nil - - Automations::Register - .actions - .detect do |a| - klass = a.for(key) - end - - klass ||= Automations::Actions::Inexistent - - klass.new(values) - end - end - - def dump(actions) - YAML::dump(actions.map { |a| [a.key, a.values.map(&:to_s)] }) - end -end diff --git a/app/models/automations/actions/strategies/date.rb b/app/models/automations/actions/strategies/date.rb index 2b80e9fe16e..040748401df 100644 --- a/app/models/automations/actions/strategies/date.rb +++ b/app/models/automations/actions/strategies/date.rb @@ -38,7 +38,7 @@ module Automations::Actions::Strategies::Date end def apply(work_package) - accessor = :"#{self.class.key}=" + accessor = :"#{key}=" if work_package.respond_to? accessor work_package.send(accessor, date_to_apply) end diff --git a/app/models/automations/actions/strategies/me_associated.rb b/app/models/automations/actions/strategies/me_associated.rb index 9d36a8fb6d5..0a5e7a2e070 100644 --- a/app/models/automations/actions/strategies/me_associated.rb +++ b/app/models/automations/actions/strategies/me_associated.rb @@ -40,7 +40,7 @@ module Automations::Actions::Strategies::MeAssociated end def values=(values) - values = Array(values).map do |v| + cast = Array(values).map do |v| if v == current_user_value_key v else @@ -48,7 +48,7 @@ module Automations::Actions::Strategies::MeAssociated end end - @values = values.uniq + write_raw_values(cast.uniq) end ## diff --git a/app/services/automations/base_service.rb b/app/services/automations/base_service.rb index cb5e6fda85d..3c5f8efc6ba 100644 --- a/app/services/automations/base_service.rb +++ b/app/services/automations/base_service.rb @@ -10,7 +10,7 @@ class Automations::BaseService &) set_attributes(action, attributes) - contract = Automations::CuContract.new(action) + contract = Automations::CuContract.new(action, user) result = ServiceResult.new(success: contract.validate && action.save, result: action, errors: contract.errors) @@ -31,36 +31,31 @@ class Automations::BaseService set_triggers(action, triggers_attributes) if triggers_attributes end - def set_actions(action, actions_attributes) - existing_action_keys = action.actions.map(&:key) + def set_actions(automation, actions_attributes) + existing_by_key = automation.actions.index_by(&:key) + incoming_keys = actions_attributes.keys.map(&:to_sym) - remove_actions(action, existing_action_keys - actions_attributes.keys) - update_actions(action, actions_attributes.slice(*existing_action_keys)) - add_actions(action, actions_attributes.slice(*(actions_attributes.keys - existing_action_keys))) + (existing_by_key.keys - incoming_keys).each do |key| + existing_by_key[key].mark_for_destruction + end + + actions_attributes.each do |key, values| + key = key.to_sym + if (existing = existing_by_key[key]) + existing.values = values + else + add_action(automation, key, values) + end + end end - def remove_actions(action, keys) - keys.each { |key| remove_action(action, key) } - end + def add_action(automation, key, values) + template = automation.available_actions.detect { |a| a.key == key } || + Automations::Actions::Inexistent.new - def update_actions(action, key_values) - key_values.each { |key, values| update_action(action, key, values) } - end - - def add_actions(action, key_values) - key_values.each { |key, values| add_action(action, key, values) } - end - - def update_action(action, key, values) - action.actions.detect { |a| a.key == key }.values = values - end - - def add_action(action, key, values) - action.actions << available_action_for(action, key).new(values) - end - - def remove_action(action, key) - action.actions.reject! { |a| a.key == key } + new_action = template.dup + new_action.values = values + automation.actions << new_action end def set_conditions(action, conditions_attributes) @@ -69,10 +64,6 @@ class Automations::BaseService end end - def available_action_for(action, key) - action.available_actions.detect { |a| a.key == key } || Automations::Actions::Inexistent - end - def available_condition_for(action, key) action.available_conditions.detect { |a| a.key == key } || Automations::Conditions::Inexistent end diff --git a/db/migrate/20260511110000_convert_custom_actions_to_automations.rb b/db/migrate/20260511110000_convert_custom_actions_to_automations.rb index 40e05ac1196..a9c57b8141d 100644 --- a/db/migrate/20260511110000_convert_custom_actions_to_automations.rb +++ b/db/migrate/20260511110000_convert_custom_actions_to_automations.rb @@ -1,12 +1,51 @@ # frozen_string_literal: true class ConvertCustomActionsToAutomations < ActiveRecord::Migration[8.1] + ACTION_KEY_TO_STI = { + "assigned_to" => "Automations::Actions::AssignedTo", + "responsible" => "Automations::Actions::Responsible", + "status" => "Automations::Actions::Status", + "priority" => "Automations::Actions::Priority", + "type" => "Automations::Actions::Type", + "project" => "Automations::Actions::Project", + "notify" => "Automations::Actions::Notify", + "done_ratio" => "Automations::Actions::DoneRatio", + "estimated_hours" => "Automations::Actions::EstimatedHours", + "start_date" => "Automations::Actions::StartDate", + "due_date" => "Automations::Actions::DueDate", + "date" => "Automations::Actions::Date" + }.freeze + + CUSTOM_FIELD_FORMAT_TO_STI = { + "string" => "Automations::Actions::CustomField::ForString", + "text" => "Automations::Actions::CustomField::ForText", + "link" => "Automations::Actions::CustomField::ForLink", + "int" => "Automations::Actions::CustomField::ForInteger", + "float" => "Automations::Actions::CustomField::ForFloat", + "date" => "Automations::Actions::CustomField::ForDate", + "bool" => "Automations::Actions::CustomField::ForBoolean", + "user" => "Automations::Actions::CustomField::ForUser", + "list" => "Automations::Actions::CustomField::ForAssociated", + "version" => "Automations::Actions::CustomField::ForAssociated" + }.freeze + class MigrationAutomation < ApplicationRecord self.table_name = "automations" end class MigrationTrigger < ApplicationRecord self.table_name = "automation_triggers" + self.inheritance_column = nil + end + + class MigrationAction < ApplicationRecord + self.table_name = "automation_actions" + self.inheritance_column = nil + end + + class MigrationCustomField < ApplicationRecord + self.table_name = "custom_fields" + self.inheritance_column = nil end def up @@ -26,8 +65,18 @@ class ConvertCustomActionsToAutomations < ActiveRecord::Migration[8.1] t.timestamps end + create_table :automation_actions do |t| + t.references :automation, null: false, foreign_key: true, index: true + t.string :type, null: false + t.jsonb :options, null: false, default: {} + t.integer :position + + t.timestamps + end + MigrationAutomation.reset_column_information MigrationTrigger.reset_column_information + MigrationAction.reset_column_information MigrationAutomation.find_each do |automation| MigrationTrigger.create!( @@ -36,10 +85,29 @@ class ConvertCustomActionsToAutomations < ActiveRecord::Migration[8.1] options: { button_label: automation.name }, position: 1 ) + + backfill_actions(automation) end + + remove_column :automations, :actions end def down + add_column :automations, :actions, :text + + MigrationAutomation.reset_column_information + + MigrationAutomation.find_each do |automation| + entries = MigrationAction + .where(automation_id: automation.id) + .order(:position, :id) + .pluck(:type, :options) + .filter_map { |type, options| serialize_action(type, options) } + + automation.update_column(:actions, YAML.dump(entries)) + end + + drop_table :automation_actions drop_table :automation_triggers rename_habtm_table :automations_projects, :custom_actions_projects, :automation_id, :custom_action_id @@ -52,6 +120,58 @@ class ConvertCustomActionsToAutomations < ActiveRecord::Migration[8.1] private + def backfill_actions(automation) + raw = automation[:actions] + return if raw.blank? + + parsed = YAML.safe_load(raw, permitted_classes: [Symbol, Date, ActiveSupport::HashWithIndifferentAccess]) + return unless parsed.is_a?(Array) + + parsed.each_with_index do |entry, index| + key, values = entry + type, options = resolve_action(key, values) + next unless type + + MigrationAction.create!( + automation_id: automation.id, + type: type, + options: options, + position: index + 1 + ) + end + end + + def resolve_action(key, values) + key_str = key.to_s + if (sti = ACTION_KEY_TO_STI[key_str]) + [sti, { values: Array(values) }] + elsif (match = key_str.match(/\Acustom_field_(\d+)\z/)) + resolve_custom_field_action(match[1].to_i, values) + end + end + + def resolve_custom_field_action(custom_field_id, values) + cf = MigrationCustomField.find_by(id: custom_field_id) + return unless cf + + sti = CUSTOM_FIELD_FORMAT_TO_STI[cf.field_format] + return unless sti + + [sti, { custom_field_id: custom_field_id, values: Array(values) }] + end + + def serialize_action(type, options) + options = options.is_a?(Hash) ? options.with_indifferent_access : {} + values = Array(options["values"]).map(&:to_s) + + if (key = ACTION_KEY_TO_STI.invert[type]) + [key, values] + elsif type.start_with?("Automations::Actions::CustomField::") + cf_id = options["custom_field_id"] + ["custom_field_#{cf_id}", values] if cf_id + end + end + def rename_habtm_table(from, to, old_fk, new_fk) rename_table from, to rename_column to, old_fk, new_fk diff --git a/spec/contracts/automations/cu_contract_spec.rb b/spec/contracts/automations/cu_contract_spec.rb index bccd37e6f72..a5b48f41bbb 100644 --- a/spec/contracts/automations/cu_contract_spec.rb +++ b/spec/contracts/automations/cu_contract_spec.rb @@ -36,10 +36,9 @@ RSpec.describe Automations::CuContract do let(:user) { build_stubbed(:user) } let(:action) do - build_stubbed(:automation, actions: - [Automations::Actions::AssignedTo.new]) + build(:automation, actions: [Automations::Actions::AssignedTo.new]) end - let(:contract) { described_class.new(action) } + let(:contract) { described_class.new(action, user) } describe "name" do it "is writable" do @@ -79,13 +78,13 @@ RSpec.describe Automations::CuContract do end it "requires a value if the action requires one" do - action.actions = [Automations::Actions::Status.new([])] + action.actions = [Automations::Actions::Status.new(values: [])] expect_contract_invalid actions: :empty end it "allows only the allowed values" do - status_action = Automations::Actions::Status.new([0]) + status_action = Automations::Actions::Status.new(values: [0]) allow(status_action) .to receive(:allowed_values) .and_return([{ value: nil, label: "-" }, diff --git a/spec/features/work_packages/automations/automations_spec.rb b/spec/features/work_packages/automations/automations_spec.rb index 24fff29eb88..cd4b294ef13 100644 --- a/spec/features/work_packages/automations/automations_spec.rb +++ b/spec/features/work_packages/automations/automations_spec.rb @@ -538,7 +538,7 @@ RSpec.describe "Automations", :js, with_ee: %i[automations] do before do create(:automation, - actions: [Automations::Actions::AssignedTo.new(value: nil)], + actions: [Automations::Actions::AssignedTo.new], name: "Unassign") end diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 02c7aed5742..18c18718109 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -1382,7 +1382,7 @@ RSpec.describe API::V3::WorkPackages::WorkPackageRepresenter do describe "customActions" do it "has a collection of customActions" do unassign_action = build_stubbed(:automation, - actions: [Automations::Actions::AssignedTo.new(value: nil)], + actions: [Automations::Actions::AssignedTo.new], name: "Unassign") allow(work_package) .to receive(:automations) @@ -1497,7 +1497,7 @@ RSpec.describe API::V3::WorkPackages::WorkPackageRepresenter do describe "customActions" do it "has an array of customActions" do unassign_action = build_stubbed(:automation, - actions: [Automations::Actions::AssignedTo.new(value: nil)], + actions: [Automations::Actions::AssignedTo.new], name: "Unassign") allow(work_package) .to receive(:automations) diff --git a/spec/models/automation_spec.rb b/spec/models/automation_spec.rb index 534a207797b..e6de3b9f825 100644 --- a/spec/models/automation_spec.rb +++ b/spec/models/automation_spec.rb @@ -32,6 +32,7 @@ require "spec_helper" RSpec.describe Automation do let(:stubbed_instance) { build_stubbed(:automation) } + let(:in_memory_instance) { build(:automation) } let(:instance) { create(:automation, name: "zzzzzzzzz") } let(:other_instance) { create(:automation, name: "aaaaa") } @@ -95,14 +96,14 @@ RSpec.describe Automation do end it "can be set and read" do - stubbed_instance.actions = [Automations::Actions::AssignedTo.new(1)] + in_memory_instance.actions = [Automations::Actions::AssignedTo.new(values: [1])] - expect(stubbed_instance.actions.map { |a| [a.key, a.values] }) + expect(in_memory_instance.actions.map { |a| [a.key, a.values] }) .to contain_exactly([:assigned_to, [1]]) end it "can be persisted" do - instance.actions = [Automations::Actions::AssignedTo.new(1)] + instance.actions = [Automations::Actions::AssignedTo.new(values: [1])] instance.save! @@ -118,9 +119,9 @@ RSpec.describe Automation do end it "returns the activated actions with their selected value and all other with the default value" do - stubbed_instance.actions = [Automations::Actions::AssignedTo.new(1)] + in_memory_instance.actions = [Automations::Actions::AssignedTo.new(values: [1])] - expect(stubbed_instance.all_actions.map { |a| [a.key, a.values] }) + expect(in_memory_instance.all_actions.map { |a| [a.key, a.values] }) .to include([:assigned_to, [1]], [:status, []]) end diff --git a/spec/models/automations/actions/custom_field_spec.rb b/spec/models/automations/actions/custom_field_spec.rb index 1960ac7fbd9..412075b8a88 100644 --- a/spec/models/automations/actions/custom_field_spec.rb +++ b/spec/models/automations/actions/custom_field_spec.rb @@ -89,42 +89,35 @@ RSpec.describe Automations::Actions::CustomField do let(:klass) do allow(WorkPackageCustomField) .to receive(:find_by) - .with(id: custom_field.id.to_s) + .with(id: custom_field.id) .and_return(custom_field) - described_class.for(custom_field.attribute_name) + described_class.subclass_for(custom_field) end let(:instance) do - klass.new + klass.new(custom_field_id: custom_field.id) end - describe ".all" do + describe ".templates" do before do allow(WorkPackageCustomField) .to receive(:usable_as_automation) .and_return(custom_fields) end - it "is an array with a list of subclasses for every custom_field" do - expect(described_class.all.length) + it "is an array of template instances for every custom_field" do + expect(described_class.templates.length) .to eql custom_fields.length - expect(described_class.all.map(&:custom_field)) + expect(described_class.templates.map(&:custom_field)) .to match_array(custom_fields) - described_class.all.each do |subclass| - expect(subclass.ancestors).to include(described_class) + described_class.templates.each do |template| + expect(template).to be_a(described_class) end end end - describe ".key" do - it "is the custom field accessor" do - expect(klass.key) - .to eql(custom_field.attribute_getter) - end - end - describe "#key" do it "is the custom field accessor" do expect(instance.key) @@ -134,7 +127,7 @@ RSpec.describe Automations::Actions::CustomField do describe "#value" do it "can be provided on initialization" do - i = klass.new(1) + i = klass.new(custom_field_id: custom_field.id, values: [1]) expect(i.values) .to eql [1] @@ -683,7 +676,7 @@ RSpec.describe Automations::Actions::CustomField do it "adds the custom value to custom_values_to_validate when applying the action" do # Create the action instance for our created custom field - action_instance = described_class.for(custom_field.attribute_name).new + action_instance = described_class.subclass_for(custom_field).new(custom_field_id: custom_field.id) action_instance.values = ["test value"] # Initially, custom_values_to_validate should be empty for persisted work packages @@ -699,7 +692,7 @@ RSpec.describe Automations::Actions::CustomField do it "does not add to custom_values_to_validate if work package doesn't respond to the setter" do # Create a work package that doesn't have this custom field other_work_package = create(:work_package) - action_instance = described_class.for(custom_field.attribute_name).new + action_instance = described_class.subclass_for(custom_field).new(custom_field_id: custom_field.id) action_instance.values = ["test value"] expect(other_work_package.custom_values_to_validate).to be_empty @@ -712,7 +705,7 @@ RSpec.describe Automations::Actions::CustomField do context "with multiple custom actions" do let(:another_custom_field) { create(:string_wp_custom_field) } - let(:another_instance) { described_class.for(another_custom_field.attribute_name).new } + let(:another_instance) { described_class.subclass_for(another_custom_field).new(custom_field_id: another_custom_field.id) } before do work_package.project.work_package_custom_fields << another_custom_field @@ -720,7 +713,7 @@ RSpec.describe Automations::Actions::CustomField do end it "accumulates custom values in custom_values_to_validate" do - action_instance = described_class.for(custom_field.attribute_name).new + action_instance = described_class.subclass_for(custom_field).new(custom_field_id: custom_field.id) action_instance.values = ["first value"] another_instance.values = ["second value"] diff --git a/spec/models/automations/actions/strategies/user_custom_field_spec.rb b/spec/models/automations/actions/strategies/user_custom_field_spec.rb index 52eb3addc17..f38f3f0a302 100644 --- a/spec/models/automations/actions/strategies/user_custom_field_spec.rb +++ b/spec/models/automations/actions/strategies/user_custom_field_spec.rb @@ -31,8 +31,8 @@ module Automations end end - let(:user_cf_action) { Automations::Actions::CustomField.for("custom_field_#{user_cf.id}").new } - let(:multi_user_cf_action) { Automations::Actions::CustomField.for("custom_field_#{multi_user_cf.id}").new } + let(:user_cf_action) { Automations::Actions::CustomField.subclass_for(user_cf).new(custom_field_id: user_cf.id) } + let(:multi_user_cf_action) { Automations::Actions::CustomField.subclass_for(multi_user_cf).new(custom_field_id: multi_user_cf.id) } let(:single_user_work_package) { create(:work_package, project: single_user_project) } let(:multi_user_work_package) { create(:work_package, project: multi_user_project) } diff --git a/spec/models/automations/shared_expectations.rb b/spec/models/automations/shared_expectations.rb index 26a68158699..68600404217 100644 --- a/spec/models/automations/shared_expectations.rb +++ b/spec/models/automations/shared_expectations.rb @@ -65,10 +65,10 @@ RSpec.shared_examples_for "base custom action" do end end - describe ".all" do - it "is an array with the class itself" do - expect(described_class.all) - .to contain_exactly(described_class) + describe ".templates" do + it "is an array with a template instance of the class" do + expect(described_class.templates) + .to contain_exactly(an_instance_of(described_class)) end end @@ -88,7 +88,7 @@ RSpec.shared_examples_for "base custom action" do describe "#values" do it "can be provided on initialization" do - i = described_class.new(expected_value) + i = described_class.new(values: [expected_value]) expect(i.values) .to eql [expected_value] diff --git a/spec/requests/api/v3/custom_actions/custom_actions_api_spec.rb b/spec/requests/api/v3/custom_actions/custom_actions_api_spec.rb index f42ac68ad2d..03a47b697b8 100644 --- a/spec/requests/api/v3/custom_actions/custom_actions_api_spec.rb +++ b/spec/requests/api/v3/custom_actions/custom_actions_api_spec.rb @@ -48,7 +48,7 @@ RSpec.describe "API::V3::CustomActions::CustomActionsAPI" do create(:user, member_with_roles: { project => role }) end let(:action) do - create(:automation_with_manual_trigger, actions: [Automations::Actions::AssignedTo.new(nil)]) + create(:automation_with_manual_trigger, actions: [Automations::Actions::AssignedTo.new]) end let(:parameters) do { @@ -94,7 +94,7 @@ RSpec.describe "API::V3::CustomActions::CustomActionsAPI" do context "for an automation without manual trigger" do let(:action) do - create(:automation_with_manual_trigger, actions: [Automations::Actions::AssignedTo.new(nil)]).tap do |automation| + create(:automation_with_manual_trigger, actions: [Automations::Actions::AssignedTo.new]).tap do |automation| automation.triggers.first.update_column(:type, "Automations::Triggers::Base") end end @@ -229,7 +229,7 @@ RSpec.describe "API::V3::CustomActions::CustomActionsAPI" do let(:admin_role) { create(:project_role) } let(:action) do create(:automation, - actions: [Automations::Actions::AssignedTo.new(nil)], + actions: [Automations::Actions::AssignedTo.new], conditions: [Automations::Conditions::Role.new(admin_role.id)]) end diff --git a/spec/services/automations/update_service_spec.rb b/spec/services/automations/update_service_spec.rb index 7f3073c1729..d9cdef7409b 100644 --- a/spec/services/automations/update_service_spec.rb +++ b/spec/services/automations/update_service_spec.rb @@ -32,7 +32,7 @@ require "spec_helper" RSpec.describe Automations::UpdateService do let(:action) do - action = build_stubbed(:automation) + action = build(:automation) allow(action) .to receive(:save) @@ -53,7 +53,7 @@ RSpec.describe Automations::UpdateService do allow(Automations::CuContract) .to receive(:new) - .with(action) + .with(action, user) .and_return(contract_instance) allow(contract_instance) @@ -130,13 +130,14 @@ RSpec.describe Automations::UpdateService do end it "updates the actions" do - action.actions = [Automations::Actions::AssignedTo.new("1"), - Automations::Actions::Status.new("3")] + action.actions = [Automations::Actions::AssignedTo.new(values: ["1"]), + Automations::Actions::Status.new(values: ["3"])] new_actions = instance .call(attributes: { actions: { assigned_to: ["2"], priority: ["3"] } }) .result .actions + .reject(&:marked_for_destruction?) .map { |a| [a.key, a.values] } expect(new_actions)