Fixes #5002 - Core Workflow: Removing groups with re-adding some discards all permissions the user has.

Co-authored-by: Florian Liebe <fl@zammad.com>

app/models/core_workflow/result.rb

@@ -3,7 +3,9 @@
 class CoreWorkflow::Result
   include ::Mixin::HasBackends
 
-  attr_accessor :payload, :payload_backup, :user, :assets, :assets_in_result, :result, :rerun, :form_updater, :restricted_fields
+  MAX_RERUN = 25
+
+  attr_accessor :payload, :payload_backup, :user, :assets, :assets_in_result, :result, :rerun, :rerun_history, :form_updater, :restricted_fields
 
   def initialize(payload:, user:, assets: {}, assets_in_result: true, result: {}, form_updater: false)
     if payload.respond_to?(:permit!)
@@ -22,6 +24,7 @@ class CoreWorkflow::Result
     @result            = result
     @form_updater      = form_updater
     @rerun             = false
+    @rerun_history     = []
   end
 
   def attributes
@@ -193,8 +196,15 @@ class CoreWorkflow::Result
     end
   end
 
+  def rerun_loop?
+    return false if rerun_history.size < 3
+
+    rerun_history.last(3).uniq.size != 3
+  end
+
   def consider_rerun
-    if @rerun && @result[:rerun_count] < 25
+    @rerun_history << Marshal.load(Marshal.dump(@result.except(:rerun_count)))
+    if @rerun && @result[:rerun_count] < MAX_RERUN && !rerun_loop?
       @result[:rerun_count] += 1
       return run
     end

app/models/core_workflow/result/base_option.rb

@@ -58,7 +58,7 @@ class CoreWorkflow::Result::BaseOption < CoreWorkflow::Result::Backend
   end
 
   def restore_array
-    new_value = @result_object.payload_backup['params'][field] & @result_object.result[:restrict_values][field]
+    new_value = @result_object.payload_backup['params'][field].map(&:to_s) & @result_object.result[:restrict_values][field]
 
     new_value_rerun(field, new_value)
 
@@ -67,7 +67,7 @@ class CoreWorkflow::Result::BaseOption < CoreWorkflow::Result::Backend
 
   def restore_string
     new_value = @result_object.payload_backup['params'][field]
-    return if @result_object.result[:restrict_values][field].exclude?(new_value)
+    return if excluded_by_restrict_values?(new_value)
 
     new_value_rerun(field, new_value)
 

spec/models/core_workflow_spec.rb

@@ -312,4 +312,50 @@ RSpec.describe CoreWorkflow, mariadb: true, type: :model do
       expect(result[:fill_in]['body']).to be_blank
     end
   end
+
+  describe 'Core-Workflows: Removing groups with re-adding some discards all permissions the user has #5002' do
+    let(:payload) do
+      base_payload.merge('params' => { 'group_id' => Group.first.id })
+    end
+    let!(:workflow1) do
+      create(:core_workflow,
+             object:  'Ticket',
+             perform: {
+               'ticket.group_id': {
+                 operator:      'remove_option',
+                 remove_option: Group.all.map { |x| x.id.to_s },
+               },
+             })
+    end
+    let!(:workflow2) do
+      create(:core_workflow,
+             object:  'Ticket',
+             perform: {
+               'ticket.group_id': {
+                 operator:   'add_option',
+                 add_option: [Group.first.id.to_s],
+               },
+             })
+    end
+
+    before do
+      action_user.group_names_access_map = {
+        Group.first.name => %w[full],
+      }
+      workflow1
+      workflow2
+    end
+
+    it 'does readd the group' do
+      expect(result[:restrict_values]['group_id']).to eq(['', Group.first.id.to_s])
+    end
+
+    it 'does keep owners' do
+      expect(result[:restrict_values]['owner_id']).to include(action_user.id.to_s)
+    end
+
+    it 'does not endless loop because of removing and adding the same element' do
+      expect(result[:rerun_count]).to be < CoreWorkflow::Result::MAX_RERUN
+    end
+  end
 end