Browse Source

Performance: Do not return unrestricted non-relatable non-filterable fields (#4754).

Co-authored-by: Florian Liebe <fl@zammad.com>
Rolf Schmidt 1 year ago
parent
commit
10329f91d8

+ 41 - 13
app/models/core_workflow/result.rb

@@ -3,19 +3,20 @@
 class CoreWorkflow::Result
   include ::Mixin::HasBackends
 
-  attr_accessor :payload, :user, :assets, :assets_in_result, :result, :rerun, :form_updater
+  attr_accessor :payload, :user, :assets, :assets_in_result, :result, :rerun, :form_updater, :restricted_fields
 
   def initialize(payload:, user:, assets: {}, assets_in_result: true, result: {}, form_updater: false)
     raise ArgumentError, __("The required parameter 'payload->class_name' is missing.") if !payload['class_name']
     raise ArgumentError, __("The required parameter 'payload->screen' is missing.") if !payload['screen']
 
-    @payload          = payload
-    @user             = user
-    @assets           = assets
-    @assets_in_result = assets_in_result
-    @result           = result
-    @form_updater     = form_updater
-    @rerun            = false
+    @restricted_fields = {}
+    @payload           = payload
+    @user              = user
+    @assets            = assets
+    @assets_in_result  = assets_in_result
+    @result            = result
+    @form_updater      = form_updater
+    @rerun             = false
   end
 
   def attributes
@@ -45,7 +46,7 @@ class CoreWorkflow::Result
       # 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)
+      run_backend_value('set_fixed_to', field, values, skip_rerun: true, skip_mark_restricted: true)
     end
 
     set_default_only_shown_if_selectable
@@ -116,21 +117,21 @@ class CoreWorkflow::Result
     end
   end
 
-  def run_backend(field, perform_config, skip_rerun: false)
+  def run_backend(field, perform_config, skip_rerun: false, skip_mark_restricted: 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, skip_rerun: skip_rerun).run
+      result << "CoreWorkflow::Result::#{backend.classify}".constantize.new(result_object: self, field: field, perform_config: perform_config, skip_rerun: skip_rerun, skip_mark_restricted: skip_mark_restricted).run
     end
     result
   end
 
-  def run_backend_value(backend, field, value, skip_rerun: false)
+  def run_backend_value(backend, field, value, skip_rerun: false, skip_mark_restricted: false)
     perform_config = {
       'operator' => backend,
       backend    => value,
     }
 
-    run_backend(field, perform_config, skip_rerun: skip_rerun)
+    run_backend(field, perform_config, skip_rerun: skip_rerun, skip_mark_restricted: skip_mark_restricted)
   end
 
   def change_flags(flags)
@@ -151,12 +152,39 @@ class CoreWorkflow::Result
     true
   end
 
+  def workflow_restricted_fields
+    @workflow_restricted_fields ||= begin
+      result = []
+      workflows.each do |workflow|
+        fields = workflow.perform.each_with_object([]) do |(key, value), result_inner|
+          next if %w[select remove_option set_fixed_to add_option].exclude?(value['operator'])
+
+          result_inner << key.split('.')[-1]
+        end
+
+        result |= fields
+      end
+      result
+    end
+  end
+
+  def filter_restrict_values
+    @result[:restrict_values].select! do |field, _values|
+      attribute = attributes.object_elements_hash[field]
+      next if attribute && workflow_restricted_fields.exclude?(field) && !@restricted_fields[field] && !attributes.attribute_options_relation?(attribute) && !attributes.attribute_filter?(attribute)
+
+      true
+    end
+  end
+
   def consider_rerun
     if @rerun && @result[:rerun_count] < 25
       @result[:rerun_count] += 1
       return run
     end
 
+    filter_restrict_values if !@form_updater
+
     assets_in_result?
 
     @result

+ 1 - 0
app/models/core_workflow/result/add_option.rb

@@ -3,6 +3,7 @@
 class CoreWorkflow::Result::AddOption < CoreWorkflow::Result::BaseOption
   def run
     @result_object.result[:restrict_values][field] |= Array(@perform_config['add_option'])
+    mark_restricted
     true
   end
 end

+ 12 - 5
app/models/core_workflow/result/backend.rb

@@ -1,11 +1,12 @@
 # Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
 class CoreWorkflow::Result::Backend
-  def initialize(result_object:, field:, perform_config:, skip_rerun: false)
-    @result_object  = result_object
-    @field          = field
-    @perform_config = perform_config
-    @skip_rerun     = skip_rerun
+  def initialize(result_object:, field:, perform_config:, skip_rerun: false, skip_mark_restricted: false)
+    @result_object        = result_object
+    @field                = field
+    @perform_config       = perform_config
+    @skip_rerun           = skip_rerun
+    @skip_mark_restricted = skip_mark_restricted
   end
 
   def field
@@ -22,6 +23,12 @@ class CoreWorkflow::Result::Backend
     @result_object.rerun = true
   end
 
+  def mark_restricted
+    return if @skip_mark_restricted
+
+    @result_object.restricted_fields[field] = true
+  end
+
   def result(backend, field, value = nil)
     @result_object.run_backend_value(backend, field, value)
   end

+ 7 - 2
app/models/core_workflow/result/remove_option.rb

@@ -2,12 +2,17 @@
 
 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(config_value)
+    update_restrict_values
     remove_excluded_param_values
+    mark_restricted
     true
   end
 
+  def update_restrict_values
+    @result_object.result[:restrict_values][field] ||= Array(@result_object.payload['params'][field])
+    @result_object.result[:restrict_values][field] -= Array(config_value)
+  end
+
   def config_value
     result = Array(@perform_config['remove_option'])
     result -= saved_value

+ 1 - 0
app/models/core_workflow/result/select.rb

@@ -7,6 +7,7 @@ class CoreWorkflow::Result::Select < CoreWorkflow::Result::Backend
     @result_object.result[:select][field]   = select_value
     @result_object.payload['params'][field] = @result_object.result[:select][field]
     set_rerun
+    mark_restricted
     true
   end
 

+ 1 - 0
app/models/core_workflow/result/set_fixed_to.rb

@@ -8,6 +8,7 @@ class CoreWorkflow::Result::SetFixedTo < CoreWorkflow::Result::BaseOption
                                                        config_value
                                                      end
     remove_excluded_param_values
+    mark_restricted
     true
   end
 

+ 0 - 31
spec/models/core_workflow/defaults_spec.rb

@@ -298,35 +298,4 @@ RSpec.describe 'CoreWorkflow > Defaults', mariadb: true, type: :model do
       expect(result[:mandatory][field_name]).to be(false)
     end
   end
-
-  describe '.perform - Default - Restrict values for multiselect fields', db_strategy: :reset do
-    let(:field_name) { SecureRandom.uuid }
-
-    before do
-      create(:object_manager_attribute_multiselect, name: field_name, display: field_name)
-      ObjectManager::Attribute.migration_execute
-    end
-
-    context 'without saved values' do
-      it 'does return the correct list of selectable values' do
-        expect(result[:restrict_values][field_name]).to eq(['', 'key_1', 'key_2', 'key_3'])
-      end
-    end
-
-    context 'with saved values' do
-      let(:payload) do
-        base_payload.merge('params' => {
-                             'id' => ticket.id,
-                           })
-      end
-
-      before do
-        ticket.reload.update(field_name.to_sym => %w[key_2 key_3])
-      end
-
-      it 'does return the correct list of selectable values' do
-        expect(result[:restrict_values][field_name]).to eq(['', 'key_1', 'key_2', 'key_3'])
-      end
-    end
-  end
 end