From 937f17e1a3402e76404bf83ac30e0195eaa06d4b Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Wed, 27 Oct 2021 16:20:30 +0200 Subject: [PATCH] Fixes #3822 - If selected value is not part of the restriction of set_fixed_to it should recalculate it with the new value. --- app/models/core_workflow/result.rb | 15 +++++--- app/models/core_workflow/result/backend.rb | 5 ++- .../core_workflow/result/base_option.rb | 4 +- spec/models/core_workflow_spec.rb | 38 +++++++++++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/app/models/core_workflow/result.rb b/app/models/core_workflow/result.rb index 8d057f945..acb39af87 100644 --- a/app/models/core_workflow/result.rb +++ b/app/models/core_workflow/result.rb @@ -35,7 +35,12 @@ class CoreWorkflow::Result # restrict init defaults to make sure param values to removed if not allowed attributes.restrict_values_default.each do |field, values| - run_backend_value('set_fixed_to', field, values) + + # skip initial rerun to improve performance + # priority e.g. would trigger a rerun because its not set yet + # but we skip rerun here because the initial values have no logic which + # are dependent on form changes + run_backend_value('set_fixed_to', field, values, skip_rerun: true) end set_default_only_shown_if_selectable @@ -89,21 +94,21 @@ class CoreWorkflow::Result end end - def run_backend(field, perform_config) + def run_backend(field, perform_config, skip_rerun: false) result = [] Array(perform_config['operator']).each do |backend| - result << "CoreWorkflow::Result::#{backend.classify}".constantize.new(result_object: self, field: field, perform_config: perform_config).run + result << "CoreWorkflow::Result::#{backend.classify}".constantize.new(result_object: self, field: field, perform_config: perform_config, skip_rerun: skip_rerun).run end result end - def run_backend_value(backend, field, value) + def run_backend_value(backend, field, value, skip_rerun: false) perform_config = { 'operator' => backend, backend => value, } - run_backend(field, perform_config) + run_backend(field, perform_config, skip_rerun: skip_rerun) end def match_workflow(workflow) diff --git a/app/models/core_workflow/result/backend.rb b/app/models/core_workflow/result/backend.rb index 4c8452dbf..44567da09 100644 --- a/app/models/core_workflow/result/backend.rb +++ b/app/models/core_workflow/result/backend.rb @@ -1,10 +1,11 @@ # Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ class CoreWorkflow::Result::Backend - def initialize(result_object:, field:, perform_config:) + def initialize(result_object:, field:, perform_config:, skip_rerun: false) @result_object = result_object @field = field @perform_config = perform_config + @skip_rerun = skip_rerun end def field @@ -12,6 +13,8 @@ class CoreWorkflow::Result::Backend end def set_rerun + return if @skip_rerun + @result_object.rerun = true end diff --git a/app/models/core_workflow/result/base_option.rb b/app/models/core_workflow/result/base_option.rb index 5bf01932b..a18ca4962 100644 --- a/app/models/core_workflow/result/base_option.rb +++ b/app/models/core_workflow/result/base_option.rb @@ -2,8 +2,6 @@ class CoreWorkflow::Result::BaseOption < CoreWorkflow::Result::Backend def remove_excluded_param_values - return if skip? - if @result_object.payload['params'][field].is_a?(Array) remove_array elsif excluded_by_restrict_values?(@result_object.payload['params'][field]) @@ -26,7 +24,7 @@ class CoreWorkflow::Result::BaseOption < CoreWorkflow::Result::Backend end def remove_string - @result_object.payload['params'][field] = nil + @result_object.payload['params'][field] = @result_object.result[:restrict_values][field]&.first set_rerun end diff --git a/spec/models/core_workflow_spec.rb b/spec/models/core_workflow_spec.rb index 065bb5c7c..325f0d92e 100644 --- a/spec/models/core_workflow_spec.rb +++ b/spec/models/core_workflow_spec.rb @@ -1752,4 +1752,42 @@ RSpec.describe CoreWorkflow, type: :model do expect(result[:restrict_values]['owner_id']).to eq(['']) end end + + describe 'If selected value is not part of the restriction of set_fixed_to it should recalculate it with the new value #3822', db_strategy: :reset do + let(:field_name1) { SecureRandom.uuid } + let(:screens) do + { + 'create_middle' => { + 'ticket.agent' => { + 'shown' => false, + 'required' => false, + } + } + } + end + let!(:workflow1) do + create(:core_workflow, + object: 'Ticket', + perform: { "ticket.#{field_name1}" => { 'operator' => 'set_fixed_to', 'set_fixed_to' => ['key_3'] } }) + end + let!(:workflow2) do + create(:core_workflow, + object: 'Ticket', + condition_selected: { + "ticket.#{field_name1}": { + operator: 'is', + value: 'key_3', + }, + }) + end + + before do + create :object_manager_attribute_select, name: field_name1, display: field_name1, screens: screens + ObjectManager::Attribute.migration_execute + end + + it 'does select key_3 as new param value and based on this executes workflow 2' do + expect(result[:matched_workflows]).to include(workflow1.id, workflow2.id) + end + end end