diff --git a/app/controllers/sessions/collection_dashboard.rb b/app/controllers/sessions/collection_dashboard.rb index 1a3ac1b91..3cf3f24fe 100644 --- a/app/controllers/sessions/collection_dashboard.rb +++ b/app/controllers/sessions/collection_dashboard.rb @@ -7,10 +7,9 @@ module ExtraCollection def session( collections, assets, user ) return [collections, assets] if !user - item = StatsStore.search( - object: 'User', - o_id: user.id, - key: 'dashboard', + item = StatsStore.find_by( + stats_storable: user, + key: 'dashboard', ) return [collections, assets] if !item diff --git a/app/models/concerns/has_search_index_backend.rb b/app/models/concerns/has_search_index_backend.rb index 99a4d2d34..b6b9a4e1b 100644 --- a/app/models/concerns/has_search_index_backend.rb +++ b/app/models/concerns/has_search_index_backend.rb @@ -108,12 +108,17 @@ returns next if local_object.to_s == 'Ticket' local_object.new.attributes.each do |key, _value| - attribute_name = key.to_s - attribute_ref_name = local_object.search_index_attribute_ref_name(attribute_name) - attribute_class = local_object.reflect_on_association(attribute_ref_name)&.klass - + attribute_name = key.to_s next if attribute_name.blank? + + attribute_ref_name = local_object.search_index_attribute_ref_name(attribute_name) next if attribute_ref_name.blank? + + association = local_object.reflect_on_association(attribute_ref_name) + next if association.blank? + next if association.options[:polymorphic] + + attribute_class = association.klass next if attribute_class.blank? next if attribute_class != self.class diff --git a/app/models/stats_store.rb b/app/models/stats_store.rb index ca8fc135e..7f3fbcf17 100644 --- a/app/models/stats_store.rb +++ b/app/models/stats_store.rb @@ -3,62 +3,17 @@ class StatsStore < ApplicationModel include HasSearchIndexBackend include StatsStore::SearchIndex - belongs_to :stats_store_object, class_name: 'ObjectLookup', optional: true - belongs_to :related_stats_store_object, class_name: 'ObjectLookup', optional: true + + belongs_to :stats_storable, polymorphic: true store :data -=begin - - count = StatsStore.count_by_search( - object: 'User', - o_id: ticket.owner_id, - key: 'ticket:reopen', - start: Time.zone.now - 7.days, - end: Time.zone.now, - ) - -=end - - def self.count_by_search(data) - - # lookups - if data[:object] - object_id = ObjectLookup.by_name(data[:object]) - end - - StatsStore.where(stats_store_object_id: object_id, o_id: data[:o_id], key: data[:key]) - .where('created_at > ? AND created_at < ?', data[:start], data[:end]).count - end - -=begin - - item = StatsStore.search( - object: 'User', - o_id: current_user.id, - key: 'dashboard', - ) - -=end - - def self.search(data) - - # lookups - if data[:object] - data[:stats_store_object_id] = ObjectLookup.by_name(data[:object]) - data.delete(:object) - end - - find_by(data) - end - =begin item = StatsStore.sync( - object: 'User', - o_id: current_user.id, - key: 'dashboard', - data: {some data}, + stats_storable: current_user, + key: 'dashboard', + data: {some data}, ) =end @@ -68,7 +23,7 @@ class StatsStore < ApplicationModel data = params[:data] params.delete(:data) - item = search(params) + item = find_by(params) if item item.data = data @@ -76,74 +31,11 @@ class StatsStore < ApplicationModel return item end - # lookups - if data[:object] - data[:stats_store_object_id] = ObjectLookup.by_name(data[:object]) - data.delete(:object) - end - params[:data] = data params[:created_by_id] = 1 create(params) end -=begin - - StatsStore.add( - object: 'User', - o_id: ticket.owner_id, - key: 'ticket:reopen', - data: { ticket_id: ticket.id }, - created_at: Time.zone.now, - ) - -=end - - def self.add(data) - - # lookups - if data[:object] - object_id = ObjectLookup.by_name(data[:object]) - end - - # create history - record = { - stats_store_object_id: object_id, - o_id: data[:o_id], - key: data[:key], - data: data[:data], - created_at: data[:created_at], - created_by_id: data[:created_by_id], - } - - StatsStore.create(record) - end - -=begin - - StatsStore.remove( - object: 'User', - o_id: ticket.owner_id, - ) - -=end - - def self.remove(data) - - # lookups - if data[:object] - object_id = ObjectLookup.by_name(data[:object]) - end - - # create history - record = { - stats_store_object_id: object_id, - o_id: data[:o_id], - } - - StatsStore.where(record).destroy_all - end - =begin cleanup old stats store diff --git a/app/models/user.rb b/app/models/user.rb index f78aa5f8b..1b49a9d0b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1164,10 +1164,7 @@ raise 'Minimum one user need to have admin permissions' def destroy_longer_required_objects ::Avatar.remove(self.class.to_s, id) ::UserDevice.remove(id) - ::StatsStore.remove( - object: self.class.to_s, - o_id: id, - ) + ::StatsStore.where(stats_storable: self).destroy_all end def destroy_move_dependency_ownership diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 7f9696eb4..41b51b3dc 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -682,17 +682,13 @@ class CreateBase < ActiveRecord::Migration[4.2] add_foreign_key :cti_caller_ids, :users create_table :stats_stores do |t| - t.references :stats_store_object, null: false - t.integer :o_id, null: false + t.references :stats_storable, polymorphic: true, index: true t.string :key, limit: 250, null: true - t.integer :related_stats_store_object_id, null: true t.string :data, limit: 5000, null: true t.integer :created_by_id, null: false t.timestamps limit: 3, null: false end - add_index :stats_stores, [:o_id] add_index :stats_stores, [:key] - add_index :stats_stores, [:stats_store_object_id] add_index :stats_stores, [:created_by_id] add_index :stats_stores, [:created_at] add_foreign_key :stats_stores, :users, column: :created_by_id diff --git a/db/migrate/20201006131231_stats_store_polymorphic_association.rb b/db/migrate/20201006131231_stats_store_polymorphic_association.rb new file mode 100644 index 000000000..8dfc163d3 --- /dev/null +++ b/db/migrate/20201006131231_stats_store_polymorphic_association.rb @@ -0,0 +1,27 @@ +class StatsStorePolymorphicAssociation < ActiveRecord::Migration[5.2] + def change + return if !Setting.exists?(name: 'system_init_done') + + # create ObjectLookup ID -> Model map + object_lookup_map = ObjectLookup.all.pluck(:id, :name) + + # create empty, indexed polymorphic association columns + add_reference :stats_stores, :stats_storable, polymorphic: true, index: true + + # migrate column data in the most performance way + object_lookup_map.each do |id, model| + StatsStore.where(stats_store_object_id: id) + .update_all("stats_storable_id = o_id, stats_storable_type = '#{model}'") # rubocop:disable Rails/SkipsModelValidations + end + + # rubocop:disable Rails/ReversibleMigration + # remove home made "polymorphic association" columns + remove_column :stats_stores, :o_id + remove_column :stats_stores, :stats_store_object_id + + # remove unused/obsolete columns + remove_column :stats_stores, :related_stats_store_object_id + remove_column(:stats_stores, :related_o_id) if StatsStore.column_names.include?('related_o_id') + # rubocop:enable Rails/ReversibleMigration + end +end diff --git a/lib/stats.rb b/lib/stats.rb index 6be0fdda6..99c7b60ca 100644 --- a/lib/stats.rb +++ b/lib/stats.rb @@ -45,7 +45,7 @@ returns data[backend] = backend.generate(user) end - user_result[user.id] = data + user_result[user] = data end # calculate average @@ -64,27 +64,26 @@ returns # generate average param and icon state backend_average_sum.each do |backend_model_average, result| average = ( result.to_f / agent_count ).round(1) - user_result.each do |user_id, data| + user_result.each do |user, data| next if !data[backend_model_average] next if !data[backend_model_average].key?(:used_for_average) data[backend_model_average][:average_per_agent] = average # generate icon state - backend_model_average.to_s.constantize.average_state(data[backend_model_average], user_id) + backend_model_average.to_s.constantize.average_state(data[backend_model_average], user.id) end end - user_result.each do |user_id, data| + user_result.each do |user, data| data_for_user = {} data.each do |backend, result| data_for_user[backend.to_app_model] = result end state_store = StatsStore.sync( - object: 'User', - o_id: user_id, - key: 'dashboard', - data: data_for_user, + stats_storable: user, + key: 'dashboard', + data: data_for_user, ) message = { @@ -93,11 +92,11 @@ returns state_store.class.to_app_model => [state_store], }, } - Sessions.send_to(user_id, message) + Sessions.send_to(user.id, message) event = { event: 'dashboard_stats_rebuild', } - Sessions.send_to(user_id, event) + Sessions.send_to(user.id, event) end true diff --git a/lib/stats/ticket_reopen.rb b/lib/stats/ticket_reopen.rb index da18c7428..204e50a2c 100644 --- a/lib/stats/ticket_reopen.rb +++ b/lib/stats/ticket_reopen.rb @@ -11,13 +11,10 @@ class Stats::TicketReopen ).count # get count of reopens - count = StatsStore.count_by_search( - object: 'User', - o_id: user.id, - key: 'ticket:reopen', - start: Time.zone.now - 7.days, - end: Time.zone.now, - ) + count = StatsStore.where( + stats_storable: user, + key: 'ticket:reopen', + ).where('created_at > ? AND created_at < ?', 7.days.ago, Time.zone.now).count if count > total total = count @@ -89,14 +86,12 @@ class Stats::TicketReopen state_type_now = Ticket::StateType.lookup(id: state_now.state_type_id) return if state_type_now.name == 'closed' - StatsStore.add( - object: 'User', - o_id: ticket.owner_id, - key: 'ticket:reopen', - data: { ticket_id: ticket.id }, - created_at: Time.zone.now, - created_by_id: updated_by_id, - updated_by_id: updated_by_id, + StatsStore.create( + stats_storable: ticket.owner, + key: 'ticket:reopen', + data: { ticket_id: ticket.id }, + created_at: Time.zone.now, + created_by_id: updated_by_id, ) end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index d96c84ad5..be511aacb 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -1257,18 +1257,16 @@ class UserTest < ActiveSupport::TestCase Token.create!(action: 'api', user_id: agent1_id) - StatsStore.add( - object: 'User', - o_id: agent1_id, - key: 'some_key', - data: { A: 1, B: 2 }, - created_at: Time.zone.now, - created_by_id: 1, + StatsStore.create( + stats_storable: agent1, + key: 'some_key', + data: { A: 1, B: 2 }, + created_at: Time.zone.now, + created_by_id: 1, ) - item = StatsStore.search( - object: 'User', - o_id: agent1_id, - key: 'some_key', + item = StatsStore.find_by( + stats_storable: agent1, + key: 'some_key', ) assert(item) @@ -1284,10 +1282,9 @@ class UserTest < ActiveSupport::TestCase assert_equal(0, RecentView.where(created_by_id: agent1_id).count) assert_equal(0, Token.where(user_id: agent1_id).count) assert_equal(0, Token.where(user_id: agent1_id).count) - item = StatsStore.search( - object: 'User', - o_id: agent1_id, - key: 'some_key', + item = StatsStore.find_by( + stats_storable: agent1, + key: 'some_key', ) assert_nil(item) end