diff --git a/app/assets/javascripts/app/controllers/_ui_element/core_workflow_perform.coffee b/app/assets/javascripts/app/controllers/_ui_element/core_workflow_perform.coffee index 4d0e3110c..b20780c62 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/core_workflow_perform.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/core_workflow_perform.coffee @@ -128,9 +128,10 @@ class App.UiElement.core_workflow_perform extends App.UiElement.ApplicationSelec @buildValueConfigMultiple: (config, meta) -> if _.contains(['add_option', 'remove_option', 'set_fixed_to'], meta.operator) config.multiple = true + config.nulloption = true else config.multiple = false - config.nulloption = false + config.nulloption = false return config @HasPreCondition: -> diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/form_handler_core_workflow.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/form_handler_core_workflow.coffee index e1548091c..d0ca39503 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/form_handler_core_workflow.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/form_handler_core_workflow.coffee @@ -119,7 +119,9 @@ class App.FormHandlerCoreWorkflow valueFound = false for value in values - if value && paramValue + + # false values are valid values e.g. for boolean fields (be careful) + if value isnt undefined && paramValue isnt undefined if value.toString() == paramValue.toString() valueFound = true break diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee index 5f033dd86..2cf626c38 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee @@ -1,12 +1,14 @@ -class Edit extends App.ControllerObserver - model: 'Ticket' - observeNot: - created_at: true - updated_at: true - globalRerender: false +# No usage of a ControllerObserver here because we want to use +# the data of the ticket zoom ajax request which is using the all=true parameter +# and contain the core workflow information as well. Without observer we also +# dont have double rendering because of the zoom (all=true) and observer (full=true) render callback +class Edit extends App.Controller + constructor: (params) -> + super + @render() - render: (ticket, diff) => - defaults = ticket.attributes() + render: => + defaults = @ticket.attributes() delete defaults.article # ignore article infos followUpPossible = App.Group.find(defaults.group_id).follow_up_possible ticketState = App.TicketState.find(defaults.state_id).name @@ -19,7 +21,7 @@ class Edit extends App.ControllerObserver if followUpPossible == 'new_ticket' && ticketState != 'closed' || followUpPossible != 'new_ticket' || - @permissionCheck('admin') || ticket.currentView() is 'agent' + @permissionCheck('admin') || @ticket.currentView() is 'agent' @controllerFormSidebarTicket = new App.ControllerForm( elReplace: @el model: { className: 'Ticket', configure_attributes: @formMeta.configure_attributes || App.Ticket.configure_attributes } @@ -28,7 +30,7 @@ class Edit extends App.ControllerObserver filter: @formMeta.filter formMeta: @formMeta params: defaults - isDisabled: !ticket.editable() + isDisabled: !@ticket.editable() taskKey: @taskKey core_workflow: { callbacks: [@markForm] @@ -44,7 +46,7 @@ class Edit extends App.ControllerObserver filter: @formMeta.filter formMeta: @formMeta params: defaults - isDisabled: ticket.editable() + isDisabled: @ticket.editable() taskKey: @taskKey core_workflow: { callbacks: [@markForm] @@ -57,8 +59,8 @@ class Edit extends App.ControllerObserver return if @resetBind @resetBind = true @controllerBind('ui::ticket::taskReset', (data) => - return if data.ticket_id.toString() isnt ticket.id.toString() - @render(ticket) + return if data.ticket_id.toString() isnt @ticket.id.toString() + @render() ) class SidebarTicket extends App.Controller @@ -128,6 +130,7 @@ class SidebarTicket extends App.Controller @edit = new Edit( object_id: @ticket.id + ticket: @ticket el: localEl.find('.edit') taskGet: @taskGet formMeta: @formMeta diff --git a/app/models/core_workflow/attributes.rb b/app/models/core_workflow/attributes.rb index 0cb4da236..7005155af 100644 --- a/app/models/core_workflow/attributes.rb +++ b/app/models/core_workflow/attributes.rb @@ -30,10 +30,26 @@ class CoreWorkflow::Attributes end end + def selectable_field?(key) + return if key == 'id' + return if !@payload['params'].key?(key) + + # some objects have no attributes like "CoreWorkflow"-object as well. + # attributes only exists in the frontend so we skip this check + return true if object_elements.blank? + + object_elements_hash.key?(key) + end + def overwrite_selected(result) selected_attributes = selected_only.attributes selected_attributes.each_key do |key| - next if selected_attributes[key].nil? + next if !selectable_field?(key) + + # special behaviour for owner id + if key == 'owner_id' && selected_attributes[key].nil? + selected_attributes[key] = 1 + end result[key.to_sym] = selected_attributes[key] end @@ -55,7 +71,10 @@ class CoreWorkflow::Attributes # dont use lookup here because the cache will not # know about new attributes and make crashes @saved_only ||= payload_class.find_by(id: @payload['params']['id']) - @saved_only.dup + + # we use marshal here because clone still uses references and dup can't + # detect changes for the rails object + Marshal.load(Marshal.dump(@saved_only)) end def saved @@ -68,6 +87,10 @@ class CoreWorkflow::Attributes end end + def object_elements_hash + @object_elements_hash ||= object_elements.index_by { |x| x[:name] } + end + def screen_value(attribute, type) attribute[:screens].dig(@payload['screen'], type) end diff --git a/app/models/core_workflow/result/backend.rb b/app/models/core_workflow/result/backend.rb index 0a458f4b6..4c8452dbf 100644 --- a/app/models/core_workflow/result/backend.rb +++ b/app/models/core_workflow/result/backend.rb @@ -18,4 +18,26 @@ class CoreWorkflow::Result::Backend def result(backend, field, value = nil) @result_object.run_backend_value(backend, field, value) end + + def saved_value + + # make sure we have a saved object + return if @result_object.attributes.saved_only.blank? + + # we only want to have the saved value in the restrictions + # if no changes happend to the form. If the users does changes + # to the form then also the saved value should get removed + return if @result_object.attributes.selected.changed? + + # attribute can be blank e.g. in custom development + # or if attribute is only available in the frontend but not + # in the backend + return if attribute.blank? + + @result_object.attributes.saved_attribute_value(attribute).to_s + end + + def attribute + @attribute ||= @result_object.attributes.object_elements_hash[field] + end end diff --git a/app/models/core_workflow/result/remove_option.rb b/app/models/core_workflow/result/remove_option.rb index a75e7f06c..03b0bcb7e 100644 --- a/app/models/core_workflow/result/remove_option.rb +++ b/app/models/core_workflow/result/remove_option.rb @@ -3,8 +3,14 @@ class CoreWorkflow::Result::RemoveOption < CoreWorkflow::Result::BaseOption def run @result_object.result[:restrict_values][field] ||= Array(@result_object.payload['params'][field]) - @result_object.result[:restrict_values][field] -= Array(@perform_config['remove_option']) + @result_object.result[:restrict_values][field] -= Array(config_value) remove_excluded_param_values true end + + def config_value + result = Array(@perform_config['remove_option']) + result -= Array(saved_value) + result + end end diff --git a/app/models/core_workflow/result/set_fixed_to.rb b/app/models/core_workflow/result/set_fixed_to.rb index 45c481b10..9203f12e3 100644 --- a/app/models/core_workflow/result/set_fixed_to.rb +++ b/app/models/core_workflow/result/set_fixed_to.rb @@ -5,21 +5,23 @@ class CoreWorkflow::Result::SetFixedTo < CoreWorkflow::Result::BaseOption @result_object.result[:restrict_values][field] = if restriction_set? restrict_values else - replace_values + config_value end remove_excluded_param_values true end + def config_value + result = Array(@perform_config['set_fixed_to']) + result |= Array(saved_value) + result + end + def restriction_set? @result_object.result[:restrict_values][field] end def restrict_values - @result_object.result[:restrict_values][field].reject { |v| Array(@perform_config['set_fixed_to']).exclude?(v) } - end - - def replace_values - Array(@perform_config['set_fixed_to']) + @result_object.result[:restrict_values][field].reject { |v| config_value.exclude?(v) } end end diff --git a/spec/system/examples/core_workflow_examples.rb b/spec/system/examples/core_workflow_examples.rb index b507bd2a8..5635bee28 100644 --- a/spec/system/examples/core_workflow_examples.rb +++ b/spec/system/examples/core_workflow_examples.rb @@ -525,15 +525,15 @@ RSpec.shared_examples 'core workflow' do perform: { "#{object_name.downcase}.#{field_name}": { operator: 'set_fixed_to', - set_fixed_to: %w[true] + set_fixed_to: %w[false] }, }) end it 'does perform' do before_it.call - expect(page).to have_selector("select[name='#{field_name}'] option[value='true']", wait: 10) - expect(page).to have_no_selector("select[name='#{field_name}'] option[value='false']", wait: 10) + expect(page).to have_selector("select[name='#{field_name}'] option[value='false']", wait: 10) + expect(page).to have_no_selector("select[name='#{field_name}'] option[value='true']", wait: 10) end end @@ -562,7 +562,7 @@ RSpec.shared_examples 'core workflow' do perform: { "#{object_name.downcase}.#{field_name}": { operator: 'set_fixed_to', - set_fixed_to: ['', 'true'], + set_fixed_to: ['', 'false'], }, }) create(:core_workflow, @@ -577,7 +577,7 @@ RSpec.shared_examples 'core workflow' do it 'does perform' do before_it.call - expect(page).to have_selector("select[name='#{field_name}'] option[value='true'][selected]", wait: 10) + expect(page).to have_selector("select[name='#{field_name}'] option[value='false'][selected]", wait: 10) end end end diff --git a/spec/system/ticket/zoom_spec.rb b/spec/system/ticket/zoom_spec.rb index 588b9ef3e..ebcf07f0b 100644 --- a/spec/system/ticket/zoom_spec.rb +++ b/spec/system/ticket/zoom_spec.rb @@ -2083,4 +2083,55 @@ RSpec.describe 'Ticket zoom', type: :system do expect(ticket.reload.owner_id).to eq(admin.id) end end + + describe "escaped 'Set fixed' workflows don't refresh set values on active ticket sessions #3757", authenticated_as: :authenticate, db_strategy: :reset do + let(:field_name) { SecureRandom.uuid } + let(:ticket) { create(:ticket, group: Group.find_by(name: 'Users'), field_name => false) } + + def authenticate + workflow + create :object_manager_attribute_boolean, name: field_name, display: field_name, screens: attributes_for(:required_screen) + ObjectManager::Attribute.migration_execute + ticket + true + end + + before do + visit "#ticket/zoom/#{ticket.id}" + end + + context 'when operator set_fixed_to' do + let(:workflow) do + create(:core_workflow, + object: 'Ticket', + perform: { "ticket.#{field_name}" => { 'operator' => 'set_fixed_to', 'set_fixed_to' => ['false'] } }) + end + + context 'when saved value is removed by set_fixed_to operator' do + it 'does show up the saved value if it would not be possible because of the restriction' do + expect(page.find("select[name='#{field_name}']").value).to eq('false') + ticket.update(field_name => true) + wait(10, interval: 0.5).until { page.find("select[name='#{field_name}']").value == 'true' } + expect(page.find("select[name='#{field_name}']").value).to eq('true') + end + end + end + + context 'when operator remove_option' do + let(:workflow) do + create(:core_workflow, + object: 'Ticket', + perform: { "ticket.#{field_name}" => { 'operator' => 'remove_option', 'remove_option' => ['true'] } }) + end + + context 'when saved value is removed by set_fixed_to operator' do + it 'does show up the saved value if it would not be possible because of the restriction' do + expect(page.find("select[name='#{field_name}']").value).to eq('false') + ticket.update(field_name => true) + wait(10, interval: 0.5).until { page.find("select[name='#{field_name}']").value == 'true' } + expect(page.find("select[name='#{field_name}']").value).to eq('true') + end + end + end + end end