Maintenance: Core Workflow - Do not show attributes without matching screen permission options.

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

app/models/core_workflow/attributes.rb

@@ -122,7 +122,7 @@ class CoreWorkflow::Attributes
 
   def object_elements_class
     ObjectManager::Object.new(@payload['class_name']).attributes(@user, saved_only, data_only: false).each_with_object([]) do |element, result|
-      result << element.data.merge(screens: element.screens)
+      result << element_data(element)
     end
   end
 
@@ -132,10 +132,14 @@ class CoreWorkflow::Attributes
     return [] if @payload['class_name'] != 'Ticket' || @payload['screen'] != 'create_middle'
 
     ObjectManager::Object.new('TicketArticle').attributes(@user, saved_only, data_only: false).each_with_object([]) do |element, result|
-      result << element.data.merge(screens: element.screens)
+      result << element_data(element)
     end.select { |o| o[:name] == 'body' }
   end
 
+  def element_data(element)
+    element.data.merge(screens: element.screens, configured_screens: element.attribute.screens)
+  end
+
   def object_elements_hash
     @object_elements_hash ||= object_elements.index_by { |x| x[:name] }
   end
@@ -154,10 +158,17 @@ class CoreWorkflow::Attributes
   # dont cache this else the result object will work with references and cache bugs occur
   def visibility_default
     object_elements.to_h do |attribute|
-      [attribute[:name], screen_value(attribute, 'shown') == false ? 'remove' : 'show']
+      [attribute[:name], attribute_shown?(attribute) ? 'show' : 'remove']
     end
   end
 
+  # A configured screen that resolves to no options is not shown, same as in the legacy frontend.
+  def attribute_shown?(attribute)
+    return false if attribute[:screens][@payload['screen']] == {} && attribute[:configured_screens][@payload['screen']].present?
+
+    screen_value(attribute, 'shown') != false
+  end
+
   def attribute_mandatory?(attribute)
     return screen_value(attribute, 'required').present? if !screen_value(attribute, 'required').nil?
     return screen_value(attribute, 'null').blank? if !screen_value(attribute, 'null').nil?

lib/user_context.rb

@@ -36,6 +36,11 @@ class UserContext < Delegator
   end
   alias kind_of? is_a?
 
+  # ActiveRecord equality checks `other.instance_of?(self.class)`, so `record.user == user_context` needs this too.
+  def instance_of?(klass)
+    super || @user.instance_of?(klass)
+  end
+
   def permissions?(permissions)
     permissions!(permissions)
     true

spec/lib/user_context_spec.rb

@@ -18,6 +18,34 @@ RSpec.describe UserContext do
     end
   end
 
+  describe '#instance_of?' do
+    let(:user)  { create(:user) }
+    let(:token) { nil }
+
+    it 'returns true for User' do
+      expect(user_context.instance_of?(User)).to be true
+    end
+
+    it 'returns false for unrelated class' do
+      expect(user_context.instance_of?(String)).to be false
+    end
+  end
+
+  describe 'ActiveRecord equality' do
+    let(:user)  { create(:user) }
+    let(:token) { nil }
+
+    it 'is equal to the wrapped user in both directions', :aggregate_failures do
+      expect(user == user_context).to be true
+      expect(user_context == user).to be true
+      expect([user]).to include(user_context)
+    end
+
+    it 'is not equal to another user' do
+      expect(create(:user) == user_context).to be false
+    end
+  end
+
   describe 'ActiveJob serialization' do
     let(:user)  { create(:user) }
     let(:token) { nil }

spec/models/core_workflow/attributes_spec.rb

@@ -60,6 +60,56 @@ RSpec.describe CoreWorkflow::Attributes, type: :model do
     it 'priority should be shown by default' do
       expect(result.visibility_default['priority_id']).to eq('show')
     end
+
+    context 'with an attribute which has screen options for specific permissions', db_strategy: :reset do
+      let(:field_name) { SecureRandom.uuid }
+
+      before do
+        create(:object_manager_attribute_text, name: field_name, screens: { 'create_middle' => screen_options })
+
+        ObjectManager::Attribute.migration_execute
+      end
+
+      context 'when the user has a matching permission' do
+        let(:screen_options) { { 'ticket.agent' => { 'shown' => true } } }
+
+        it 'shows the attribute' do
+          expect(result.visibility_default[field_name]).to eq('show')
+        end
+      end
+
+      context 'when the user has no matching permission' do
+        let(:screen_options) { { 'ticket.customer' => { 'shown' => true } } }
+
+        it 'removes the attribute' do
+          expect(result.visibility_default[field_name]).to eq('remove')
+        end
+      end
+
+      context 'when the matching permission has no options' do
+        let(:screen_options) { { 'ticket.agent' => {} } }
+
+        it 'removes the attribute' do
+          expect(result.visibility_default[field_name]).to eq('remove')
+        end
+      end
+
+      context 'when the -all- permission has no options' do
+        let(:screen_options) { { '-all-' => {} } }
+
+        it 'removes the attribute' do
+          expect(result.visibility_default[field_name]).to eq('remove')
+        end
+      end
+
+      context 'when the screen has no options at all' do
+        let(:screen_options) { {} }
+
+        it 'shows the attribute' do
+          expect(result.visibility_default[field_name]).to eq('show')
+        end
+      end
+    end
   end
 
   describe '#options_array' do

spec/models/core_workflow/conditions_spec.rb

@@ -881,6 +881,14 @@ RSpec.describe 'CoreWorkflow > Conditions', type: :model do
         expect(result[:matched_workflows]).not_to include(workflow.id)
       end
     end
+
+    describe 'for agent wrapped in a user context' do
+      let!(:action_user) { UserContext.new(create(:agent, groups: [ticket.group])) } # rubocop:disable RSpec/LetSetup
+
+      it 'does match' do
+        expect(result[:matched_workflows]).to include(workflow.id)
+      end
+    end
   end
 
   describe '.perform - Condition - session.group_ids_change' do

spec/models/object_manager/object_spec.rb

@@ -279,6 +279,16 @@ RSpec.describe ObjectManager::Object do
         end
       end
 
+      describe 'with a customer wrapped in a user context' do
+        let(:customer) { create(:customer) }
+        let(:user)     { UserContext.new(customer) }
+        let(:record)   { create(:ticket, customer: customer) }
+
+        it 'applies ticket.customer screen options' do
+          expect(attribute[:screen]['edit']['shown']).to be true
+        end
+      end
+
       # https://github.com/zammad/zammad/issues/5993
       describe 'with a shared organization' do
         let(:organization)   { create(:organization, shared: true) }

spec/policies/token_policy_spec.rb

@@ -19,6 +19,12 @@ describe TokenPolicy do
 
       it { is_expected.to forbid_action(:destroy) }
     end
+
+    context 'when the same user is wrapped in a user context' do
+      let(:user) { UserContext.new(record.user) }
+
+      it { is_expected.to permit_action(:destroy) }
+    end
   end
 
   context 'when token is not visible in frontend' do

spec/policies/user/overview_sorting_policy_spec.rb

@@ -21,4 +21,12 @@ describe User::OverviewSortingPolicy do
 
     it { is_expected.to forbid_actions(%i[show create update destroy]) }
   end
+
+  context 'with the user wrapped in a user context' do
+    subject { described_class.new(UserContext.new(user1), record) }
+
+    let(:record) { record1 }
+
+    it { is_expected.to permit_actions(%i[show create update destroy]) }
+  end
 end