From 5f2181d8a3e8a86acdc6ce0576343e23242c407d Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@zammad.com>
Date: Wed, 29 Sep 2021 10:03:04 +0200
Subject: [PATCH] Fixes #3757 - escaped 'Set fixed' workflows don't refresh set
 values on active ticket sessions.

---
 .../_ui_element/core_workflow_perform.coffee  |  3 +-
 .../form_handler_core_workflow.coffee         |  4 +-
 .../ticket_zoom/sidebar_ticket.coffee         | 29 ++++++-----
 app/models/core_workflow/attributes.rb        | 27 +++++++++-
 app/models/core_workflow/result/backend.rb    | 22 ++++++++
 .../core_workflow/result/remove_option.rb     |  8 ++-
 .../core_workflow/result/set_fixed_to.rb      | 14 ++---
 .../system/examples/core_workflow_examples.rb | 10 ++--
 spec/system/ticket/zoom_spec.rb               | 51 +++++++++++++++++++
 9 files changed, 139 insertions(+), 29 deletions(-)

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