Performance: Drop unnessescary user assets for users, roles and organizations.

app/models/organization/assets.rb

@@ -61,16 +61,6 @@ returns
       if !data[ app_model_user ]
         data[ app_model_user ] = {}
       end
-
-      %w[created_by_id updated_by_id].each do |local_user_id|
-        next if !self[ local_user_id ]
-        next if data[ app_model_user ][ self[ local_user_id ] ]
-
-        user = User.lookup(id: self[ local_user_id ])
-        next if !user
-
-        data = user.assets(data)
-      end
       data
     end
 

app/models/role/assets.rb

@@ -45,19 +45,6 @@ returns
 
         data = group.assets(data)
       end
-
-      return data if !self['created_by_id'] && !self['updated_by_id']
-
-      app_model_user = User.to_app_model
-      %w[created_by_id updated_by_id].each do |local_user_id|
-        next if !self[ local_user_id ]
-        next if data[ app_model_user ] && data[ app_model_user ][ self[ local_user_id ] ]
-
-        user = User.lookup(id: self[ local_user_id ])
-        next if !user
-
-        data = user.assets(data)
-      end
       data
     end
 

app/models/user/assets.rb

@@ -92,15 +92,6 @@ returns
           end
         end
       end
-      %w[created_by_id updated_by_id].each do |local_user_id|
-        next if !self[ local_user_id ]
-        next if data[ app_model ][ self[ local_user_id ] ]
-
-        user = User.lookup(id: self[ local_user_id ])
-        next if !user
-
-        data = user.assets(data)
-      end
       data
     end
 

spec/models/application_model/can_assets_examples.rb

@@ -15,15 +15,6 @@ RSpec.shared_examples 'ApplicationModel::CanAssets' do |associations: [], select
 
     include_examples 'own asset attributes' if own_attributes
 
-    describe 'for created_by & updated_by users' do
-      let(:users) { User.where(id: subject.attributes.slice('created_by_id', 'updated_by_id').values) }
-      let(:users_assets) { users.reduce({}) { |assets_hash, user| user.assets(assets_hash) } }
-
-      it 'returns a hash with their asset attributes' do
-        expect(subject.assets({})[:User]).to include(users_assets[:User])
-      end
-    end
-
     context 'when given a non-empty hash' do
       let(:hash) { { described_class.to_app_model => { foo: 'bar' } } }
 
@@ -65,7 +56,7 @@ RSpec.shared_examples 'ApplicationModel::CanAssets' do |associations: [], select
           let(:collection_assets) { collection.reduce({}) { |assets_hash, single| single.assets(assets_hash) } }
 
           it 'returns a hash with their asset attributes' do
-            expect(subject.assets({})).to include(collection_assets)
+            expect(subject.assets({})[reflection.klass.to_app_model]).to include(collection_assets[reflection.klass.to_app_model])
           end
 
           context 'after association has been modified' do

spec/models/concerns/has_history_examples.rb

@@ -117,8 +117,7 @@ RSpec.shared_examples 'HasHistory' do |history_relation_object: []|
       end
 
       it 'returns a hash including FE assets of self and related objects' do
-        expect(subject.history_get(true))
-          .to include(assets: hash_including(subject.assets({})))
+        expect(subject.history_get(true)[:assets][described_class.to_app_model]).to include(subject.assets({})[described_class.to_app_model])
       end
     end
   end

spec/requests/user_spec.rb

@@ -783,13 +783,6 @@ RSpec.describe 'User', type: :request, performs_jobs: true do
       expect(json_response['assets']['User'][user.id.to_s]['firstname']).to eq(user.firstname)
       expect(json_response['assets']['User'][user.id.to_s]['lastname']).to eq(user.lastname)
       expect(json_response['assets']['User'][user.id.to_s]['password']).to be_falsey
-
-      expect(json_response['assets']['User'][admin.id.to_s]).to be_truthy
-      expect(json_response['assets']['User'][admin.id.to_s]['id']).to eq(admin.id)
-      expect(json_response['assets']['User'][admin.id.to_s]['firstname']).to eq(admin.firstname)
-      expect(json_response['assets']['User'][admin.id.to_s]['lastname']).to eq(admin.lastname)
-      expect(json_response['assets']['User'][admin.id.to_s]['password']).to be_falsey
-
     end
 
     it 'does ticket update and response formats (04.04)' do
@@ -854,13 +847,6 @@ RSpec.describe 'User', type: :request, performs_jobs: true do
       expect(json_response['assets']['User'][user.id.to_s]['firstname']).to eq(params[:firstname])
       expect(json_response['assets']['User'][user.id.to_s]['lastname']).to eq(user.lastname)
       expect(json_response['assets']['User'][user.id.to_s]['password']).to be_falsey
-
-      expect(json_response['assets']['User'][admin.id.to_s]).to be_truthy
-      expect(json_response['assets']['User'][admin.id.to_s]['id']).to eq(admin.id)
-      expect(json_response['assets']['User'][admin.id.to_s]['firstname']).to eq(admin.firstname)
-      expect(json_response['assets']['User'][admin.id.to_s]['lastname']).to eq(admin.lastname)
-      expect(json_response['assets']['User'][admin.id.to_s]['password']).to be_falsey
-
     end
 
     it 'does csv example - customer no access (05.01)' do

test/unit/user_assets_test.rb

@@ -1,122 +0,0 @@
-# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
-
-# NOTE: This test file is _almost_ fully migrated to RSpec, as of 4cc64d0ce.
-# It may be deleted once all missing spec coverage has been added.
-#
-# What's missing is coverage for
-# the non-standard implementation of #assets on the User class.
-# (It adds an { accounts: {} } key-value pair
-# to the resulting User attributes hash;
-# see lines 75:83:91:109:123:131:139 of this file).
-#
-# This omission is discussed in detail in
-# https://git.zammad.com/zammad/zammad/merge_requests/363
-
-require 'test_helper'
-
-class UserAssetsTest < ActiveSupport::TestCase
-  test 'assets' do
-
-    roles  = Role.where(name: %w[Agent Admin])
-    groups = Group.all
-    org1   = Organization.create_or_update(
-      name:          'some user org',
-      updated_by_id: 1,
-      created_by_id: 1,
-    )
-
-    user1 = User.create_or_update(
-      login:           'assets1@example.org',
-      firstname:       'assets1',
-      lastname:        'assets1',
-      email:           'assets1@example.org',
-      password:        'some_pass',
-      active:          true,
-      updated_by_id:   1,
-      created_by_id:   1,
-      organization_id: org1.id,
-      roles:           roles,
-      groups:          groups,
-    )
-
-    user2 = User.create_or_update(
-      login:         'assets2@example.org',
-      firstname:     'assets2',
-      lastname:      'assets2',
-      email:         'assets2@example.org',
-      password:      'some_pass',
-      active:        true,
-      updated_by_id: 1,
-      created_by_id: 1,
-      roles:         roles,
-      groups:        groups,
-    )
-
-    user3 = User.create_or_update(
-      login:         'assets3@example.org',
-      firstname:     'assets3',
-      lastname:      'assets3',
-      email:         'assets3@example.org',
-      password:      'some_pass',
-      active:        true,
-      updated_by_id: user1.id,
-      created_by_id: user2.id,
-      roles:         roles,
-      groups:        groups,
-    )
-    user3 = User.find(user3.id)
-    assets = user3.assets({})
-
-    org1 = Organization.find(org1.id)
-    attributes = org1.attributes_with_association_ids
-    attributes.delete('user_ids')
-    assert(diff(attributes, assets[:Organization][org1.id]), 'check assets')
-
-    user1 = User.find(user1.id)
-    attributes = user1.attributes_with_association_ids
-    attributes['accounts'] = {}
-    attributes.delete('password')
-    attributes.delete('token_ids')
-    attributes.delete('authorization_ids')
-    assert(diff(attributes, assets[:User][user1.id]), 'check assets')
-
-    user2 = User.find(user2.id)
-    attributes = user2.attributes_with_association_ids
-    attributes['accounts'] = {}
-    attributes.delete('password')
-    attributes.delete('token_ids')
-    attributes.delete('authorization_ids')
-    assert(diff(attributes, assets[:User][user2.id]), 'check assets')
-
-    user3 = User.find(user3.id)
-    attributes = user3.attributes_with_association_ids
-    attributes['accounts'] = {}
-    attributes.delete('password')
-    attributes.delete('token_ids')
-    attributes.delete('authorization_ids')
-    assert(diff(attributes, assets[:User][user3.id]), 'check assets')
-
-    user3.destroy!
-    user2.destroy!
-    user1.destroy!
-    org1.destroy!
-  end
-
-  def diff(object1, object2)
-    return true if object1 == object2
-
-    %w[updated_at created_at].each do |item|
-      if object1[item]
-        object1[item] = object1[item].to_s
-      end
-      if object2[item]
-        object2[item] = object2[item].to_s
-      end
-    end
-    return true if (object1.to_a - object2.to_a).blank?
-
-    # puts "ERROR: difference \n1: #{object1.inspect}\n2: #{object2.inspect}\ndiff: #{(object1.to_a - object2.to_a).inspect}"
-    false
-  end
-
-end