From ffd7ae99b1a9f5a79ca0f21e29d0668d77ea65e2 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 7 Apr 2019 17:23:03 +0200 Subject: [PATCH] Fixes issue #2530 - Not used SQL index with PostgreSQL in certain cases on very large setups. --- .../application_controller/renders_models.rb | 4 +- app/controllers/http_logs_controller.rb | 4 +- app/controllers/organizations_controller.rb | 4 +- app/controllers/tags_controller.rb | 4 +- app/controllers/tickets_controller.rb | 10 +-- .../user_access_token_controller.rb | 2 +- app/controllers/user_devices_controller.rb | 2 +- app/controllers/users_controller.rb | 4 +- app/models/activity_stream.rb | 10 +-- .../application_model/can_latest_change.rb | 2 +- app/models/avatar.rb | 4 +- app/models/channel/email_parser.rb | 2 +- app/models/chat.rb | 2 +- .../concerns/has_search_index_backend.rb | 2 +- app/models/cti/caller_id.rb | 2 +- app/models/cti/log.rb | 2 +- app/models/history.rb | 4 +- app/models/karma/activity_log.rb | 2 +- app/models/online_notification.rb | 4 +- app/models/scheduler.rb | 2 +- app/models/store.rb | 4 +- app/models/ticket/article.rb | 2 +- app/models/ticket/screen_options.rb | 12 ++-- db/migrate/20120101000001_create_base.rb | 4 ++ db/migrate/20120101000010_create_ticket.rb | 8 +++ db/migrate/20190405000001_database_indexes.rb | 72 +++++++++++++++++++ lib/session_helper.rb | 2 +- lib/sessions/event/chat_session_start.rb | 2 +- lib/sessions/event/chat_status_agent.rb | 2 +- spec/models/channel/email_parser_spec.rb | 2 +- spec/models/cti/log_spec.rb | 7 +- spec/requests/integration/cti_spec.rb | 20 ++++++ spec/requests/integration/placetel_spec.rb | 28 ++++++++ spec/requests/integration/sipgate_spec.rb | 26 +++++++ test/unit/karma_test.rb | 32 ++++----- 35 files changed, 229 insertions(+), 66 deletions(-) create mode 100644 db/migrate/20190405000001_database_indexes.rb diff --git a/app/controllers/application_controller/renders_models.rb b/app/controllers/application_controller/renders_models.rb index adc305224..e6e2166f2 100644 --- a/app/controllers/application_controller/renders_models.rb +++ b/app/controllers/application_controller/renders_models.rb @@ -113,9 +113,9 @@ module ApplicationController::RendersModels end generic_objects = if offset.positive? - object.limit(params[:per_page]).order(id: 'ASC').offset(offset).limit(limit) + object.limit(params[:per_page]).order(id: :asc).offset(offset).limit(limit) else - object.all.order(id: 'ASC').offset(offset).limit(limit) + object.all.order(id: :asc).offset(offset).limit(limit) end if response_expand? diff --git a/app/controllers/http_logs_controller.rb b/app/controllers/http_logs_controller.rb index ccbe61b90..82ef3eb2c 100644 --- a/app/controllers/http_logs_controller.rb +++ b/app/controllers/http_logs_controller.rb @@ -7,9 +7,9 @@ class HttpLogsController < ApplicationController def index permission_check('admin.*') list = if params[:facility] - HttpLog.where(facility: params[:facility]).order('created_at DESC').limit(params[:limit] || 50) + HttpLog.where(facility: params[:facility]).order(created_at: :desc).limit(params[:limit] || 50) else - HttpLog.order('created_at DESC').limit(params[:limit] || 50) + HttpLog.order(created_at: :desc).limit(params[:limit] || 50) end model_index_render_result(list) end diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 8aefac109..08271edba 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -63,10 +63,10 @@ curl http://localhost/api/v1/organizations -v -u #{login}:#{password} organizations = [] if !current_user.permissions?(['admin.organization', 'ticket.agent']) if current_user.organization_id - organizations = Organization.where(id: current_user.organization_id).order(id: 'ASC').offset(offset).limit(per_page) + organizations = Organization.where(id: current_user.organization_id).order(id: :asc).offset(offset).limit(per_page) end else - organizations = Organization.all.order(id: 'ASC').offset(offset).limit(per_page) + organizations = Organization.all.order(id: :asc).offset(offset).limit(per_page) end if response_expand? diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index dbb1d6b53..9f636458a 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -5,7 +5,7 @@ class TagsController < ApplicationController # GET /api/v1/tag_search?term=abc def search - list = Tag::Item.where('name_downcase LIKE ?', "#{params[:term].strip.downcase}%").order('name ASC').limit(params[:limit] || 10) + list = Tag::Item.where('name_downcase LIKE ?', "#{params[:term].strip.downcase}%").order(name: :asc).limit(params[:limit] || 10) results = [] list.each do |item| result = { @@ -61,7 +61,7 @@ class TagsController < ApplicationController # GET /api/v1/tag_list def admin_list permission_check('admin.tag') - list = Tag::Item.order('name ASC').limit(params[:limit] || 1000) + list = Tag::Item.order(name: :asc).limit(params[:limit] || 1000) results = [] list.each do |item| result = { diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 76b98c326..20f8824ff 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -24,7 +24,7 @@ class TicketsController < ApplicationController end access_condition = Ticket.access_condition(current_user, 'read') - tickets = Ticket.where(access_condition).order(id: 'ASC').offset(offset).limit(per_page) + tickets = Ticket.where(access_condition).order(id: :asc).offset(offset).limit(per_page) if response_expand? list = [] @@ -309,11 +309,11 @@ class TicketsController < ApplicationController ticket_lists = Ticket .where( customer_id: ticket.customer_id, - state_id: Ticket::State.by_category(:open) + state_id: Ticket::State.by_category(:open).pluck(:id), ) .where(access_condition) .where('id != ?', [ ticket.id ]) - .order('created_at DESC') + .order(created_at: :desc) .limit(6) # if we do not have open related tickets, search for any tickets @@ -322,11 +322,11 @@ class TicketsController < ApplicationController .where( customer_id: ticket.customer_id, ).where.not( - state_id: Ticket::State.by_category(:merged) + state_id: Ticket::State.by_category(:merged).pluck(:id), ) .where(access_condition) .where('id != ?', [ ticket.id ]) - .order('created_at DESC') + .order(created_at: :desc) .limit(6) end diff --git a/app/controllers/user_access_token_controller.rb b/app/controllers/user_access_token_controller.rb index 4ba010106..91c252c02 100644 --- a/app/controllers/user_access_token_controller.rb +++ b/app/controllers/user_access_token_controller.rb @@ -27,7 +27,7 @@ curl http://localhost/api/v1/user_access_token -v -u #{login}:#{password} =end def index - tokens = Token.where(action: 'api', persistent: true, user_id: current_user.id).order('updated_at DESC, label ASC') + tokens = Token.where(action: 'api', persistent: true, user_id: current_user.id).order(updated_at: :desc, label: :asc) token_list = [] tokens.each do |token| attributes = token.attributes diff --git a/app/controllers/user_devices_controller.rb b/app/controllers/user_devices_controller.rb index 37b1cac4f..11169348c 100644 --- a/app/controllers/user_devices_controller.rb +++ b/app/controllers/user_devices_controller.rb @@ -4,7 +4,7 @@ class UserDevicesController < ApplicationController prepend_before_action { authentication_check(permission: 'user_preferences.device') } def index - devices = UserDevice.where(user_id: current_user.id).order('updated_at DESC, name ASC') + devices = UserDevice.where(user_id: current_user.id).order(updated_at: :desc, name: :asc) devices_full = [] devices.each do |device| attributes = device.attributes diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f39a36ddb..1c155ab53 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -29,9 +29,9 @@ class UsersController < ApplicationController # only allow customer to fetch him self users = if !current_user.permissions?(['admin.user', 'ticket.agent']) - User.where(id: current_user.id).order(id: 'ASC').offset(offset).limit(per_page) + User.where(id: current_user.id).order(id: :asc).offset(offset).limit(per_page) else - User.all.order(id: 'ASC').offset(offset).limit(per_page) + User.all.order(id: :asc).offset(offset).limit(per_page) end if response_expand? diff --git a/app/models/activity_stream.rb b/app/models/activity_stream.rb index f19120c96..76763a115 100644 --- a/app/models/activity_stream.rb +++ b/app/models/activity_stream.rb @@ -59,7 +59,7 @@ add a new activity entry for an object permission_id: permission_id, activity_stream_object_id: object_id, created_by_id: data[:created_by_id] - ).order('created_at DESC, id DESC').first + ).order(created_at: :desc).first # resturn if old entry is really fresh if result @@ -113,12 +113,12 @@ return all activity entries of an user group_ids = user.group_ids_access('read') stream = if group_ids.blank? - ActivityStream.where('(permission_id IN (?) AND group_id is NULL)', permission_ids) - .order('created_at DESC, id DESC') + ActivityStream.where('(permission_id IN (?) AND group_id IS NULL)', permission_ids) + .order(created_at: :desc) .limit(limit) else - ActivityStream.where('(permission_id IN (?) AND group_id is NULL) OR (permission_id IN (?) AND group_id IN (?)) OR (permission_id is NULL AND group_id IN (?))', permission_ids, permission_ids, group_ids, group_ids) - .order('created_at DESC, id DESC') + ActivityStream.where('(permission_id IN (?) AND (group_id IS NULL OR group_id IN (?))) OR (permission_id IS NULL AND group_id IN (?))', permission_ids, group_ids, group_ids) + .order(created_at: :desc) .limit(limit) end stream diff --git a/app/models/application_model/can_latest_change.rb b/app/models/application_model/can_latest_change.rb index 39f0c2fe2..ff66837f5 100644 --- a/app/models/application_model/can_latest_change.rb +++ b/app/models/application_model/can_latest_change.rb @@ -24,7 +24,7 @@ returns return updated_at if updated_at # if we do not have it cached, do lookup - updated_at = order(updated_at: :desc, id: :desc).limit(1).pluck(:updated_at).first + updated_at = Ticket.order(updated_at: :desc).limit(1).pluck(:updated_at).first return if !updated_at diff --git a/app/models/avatar.rb b/app/models/avatar.rb index 0d6684910..0965d2ec7 100644 --- a/app/models/avatar.rb +++ b/app/models/avatar.rb @@ -323,7 +323,7 @@ return all avatars of an user avatars = Avatar.where( object_lookup_id: object_id, o_id: o_id, - ).order('initial DESC, deletable ASC, created_at ASC, id DESC') + ).order(initial: :desc, deletable: :asc, created_at: :asc) # add initial avatar if no_init_add_as_boolean @@ -388,7 +388,7 @@ returns: avatars = Avatar.where( object_lookup_id: object_id, o_id: o_id, - ).order('created_at ASC, id DESC') + ).order(created_at: :asc) avatars.each do |avatar| next if avatar.id == avatar_id diff --git a/app/models/channel/email_parser.rb b/app/models/channel/email_parser.rb index 30bea2cb4..50955950a 100644 --- a/app/models/channel/email_parser.rb +++ b/app/models/channel/email_parser.rb @@ -231,7 +231,7 @@ returns group = Group.lookup(id: channel[:group_id]) end if group.blank? || group.active == false - group = Group.where(active: true).order('id ASC').first + group = Group.where(active: true).order(id: :asc).first end if group.blank? group = Group.first diff --git a/app/models/chat.rb b/app/models/chat.rb index d407ca804..d94bc1988 100644 --- a/app/models/chat.rb +++ b/app/models/chat.rb @@ -206,7 +206,7 @@ broadcast new customer queue position to all waiting customers # send position update to other waiting sessions position = 0 - Chat::Session.where(state: 'waiting').order('created_at ASC').each do |local_chat_session| + Chat::Session.where(state: 'waiting').order(created_at: :asc).each do |local_chat_session| position += 1 data = { event: 'chat_session_queue', diff --git a/app/models/concerns/has_search_index_backend.rb b/app/models/concerns/has_search_index_backend.rb index 1f09d77fd..31b37d412 100644 --- a/app/models/concerns/has_search_index_backend.rb +++ b/app/models/concerns/has_search_index_backend.rb @@ -130,7 +130,7 @@ reload search index with full data def search_index_reload tolerance = 5 tolerance_count = 0 - ids = all.order('created_at DESC').pluck(:id) + ids = all.order(created_at: :desc).pluck(:id) ids.each do |item_id| item = find(item_id) next if item.ignore_search_indexing?(:destroy) diff --git a/app/models/cti/caller_id.rb b/app/models/cti/caller_id.rb index 1d4c4a245..e6be826d3 100644 --- a/app/models/cti/caller_id.rb +++ b/app/models/cti/caller_id.rb @@ -53,7 +53,7 @@ returns Cti::CallerId.select('MAX(id) as caller_id') .where({ caller_id: caller_id, level: level }.compact) .group(:user_id) - .order('caller_id DESC') + .order('caller_id DESC') # not used as `caller_id: :desc` because is needed for `as caller_id` .limit(20) .map(&:caller_id) end.find(&:present?) diff --git a/app/models/cti/log.rb b/app/models/cti/log.rb index a559fcbd8..f4489ff94 100644 --- a/app/models/cti/log.rb +++ b/app/models/cti/log.rb @@ -308,7 +308,7 @@ returns =end def self.log_records - Cti::Log.order('created_at DESC, id DESC').limit(60) + Cti::Log.order(created_at: :desc).limit(60) end =begin diff --git a/app/models/history.rb b/app/models/history.rb index 23b170344..e6f96ba0c 100644 --- a/app/models/history.rb +++ b/app/models/history.rb @@ -153,7 +153,7 @@ returns history_object = object_lookup(requested_object) history = History.where(history_object_id: history_object.id) .where(o_id: requested_object_id) - .order('created_at ASC, id ASC') + .order(created_at: :asc) else history_object_requested = object_lookup(requested_object) history_object_related = object_lookup(related_history_object) @@ -164,7 +164,7 @@ returns history_object_related.id, requested_object_id, ) - .order('created_at ASC, id ASC') + .order(created_at: :asc) end asset_list = {} list = [] diff --git a/app/models/karma/activity_log.rb b/app/models/karma/activity_log.rb index 30e016b9b..b49478427 100644 --- a/app/models/karma/activity_log.rb +++ b/app/models/karma/activity_log.rb @@ -21,7 +21,7 @@ add karma activity log of an object end Karma::ActivityLog.transaction do - last_activity = Karma::ActivityLog.where(user_id: user.id).order(created_at: :desc, id: :desc).lock(true).first + last_activity = Karma::ActivityLog.where(user_id: user.id).order(id: :desc).lock(true).first latest_activity = Karma::ActivityLog.where( user_id: user.id, object_lookup_id: object_id, diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index 78825fa19..9e66afdf6 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -120,7 +120,7 @@ return all online notifications of an user def self.list(user, limit) OnlineNotification.where(user_id: user.id) - .order('created_at DESC, id DESC') + .order(created_at: :desc) .limit(limit) end @@ -138,7 +138,7 @@ return all online notifications of an object object_lookup_id: object_id, o_id: o_id, ) - .order('created_at DESC, id DESC') + .order(created_at: :desc) .limit(10_000) notifications end diff --git a/app/models/scheduler.rb b/app/models/scheduler.rb index 59262fa7f..d10551f77 100644 --- a/app/models/scheduler.rb +++ b/app/models/scheduler.rb @@ -37,7 +37,7 @@ class Scheduler < ApplicationModel end # read/load jobs and check if it is alredy started - jobs = Scheduler.where('active = ?', true).order('prio ASC') + jobs = Scheduler.where('active = ?', true).order(prio: :asc) jobs.each do |job| # ignore job is still running diff --git a/app/models/store.rb b/app/models/store.rb index d6b7ca6fe..4dd5c1a58 100644 --- a/app/models/store.rb +++ b/app/models/store.rb @@ -115,7 +115,7 @@ returns # search store_object_id = Store::Object.lookup(name: data[:object]) stores = Store.where(store_object_id: store_object_id, o_id: data[:o_id].to_i) - .order('created_at ASC, id ASC') + .order(created_at: :asc) stores end @@ -139,7 +139,7 @@ returns store_object_id = Store::Object.lookup(name: data[:object]) stores = Store.where(store_object_id: store_object_id) .where(o_id: data[:o_id]) - .order('created_at ASC, id ASC') + .order(created_at: :asc) stores.each do |store| # check backend for references diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index a85171952..240f89696 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -213,7 +213,7 @@ returns def self.last_customer_agent_article(ticket_id) sender = Ticket::Article::Sender.lookup(name: 'System') - Ticket::Article.where('ticket_id = ? AND sender_id NOT IN (?)', ticket_id, sender.id).order('created_at DESC').first + Ticket::Article.where('ticket_id = ? AND sender_id NOT IN (?)', ticket_id, sender.id).order(created_at: :desc).first end =begin diff --git a/app/models/ticket/screen_options.rb b/app/models/ticket/screen_options.rb index ebf485a26..81e566824 100644 --- a/app/models/ticket/screen_options.rb +++ b/app/models/ticket/screen_options.rb @@ -177,14 +177,14 @@ returns def self.list_by_customer(data) # get closed/open states - state_list_open = Ticket::State.by_category(:open) - state_list_closed = Ticket::State.by_category(:closed) + state_id_list_open = Ticket::State.by_category(:open).pluck(:id) + state_id_list_closed = Ticket::State.by_category(:closed).pluck(:id) # get tickets tickets_open = Ticket.where( customer_id: data[:customer_id], - state_id: state_list_open - ).limit( data[:limit] || 15 ).order('created_at DESC') + state_id: state_id_list_open + ).limit(data[:limit] || 15).order(created_at: :desc) assets = {} ticket_ids_open = [] tickets_open.each do |ticket| @@ -194,8 +194,8 @@ returns tickets_closed = Ticket.where( customer_id: data[:customer_id], - state_id: state_list_closed - ).limit( data[:limit] || 15 ).order('created_at DESC') + state_id: state_id_list_closed + ).limit(data[:limit] || 15).order(created_at: :desc) ticket_ids_closed = [] tickets_closed.each do |ticket| ticket_ids_closed.push ticket.id diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index b39974ac6..cc4141e9b 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -355,6 +355,8 @@ class CreateBase < ActiveRecord::Migration[4.2] add_index :activity_streams, [:o_id] add_index :activity_streams, [:created_by_id] add_index :activity_streams, [:permission_id] + add_index :activity_streams, %i[permission_id group_id] + add_index :activity_streams, %i[permission_id group_id created_at], name: 'index_activity_streams_on_permission_id_group_id_created_at' add_index :activity_streams, [:group_id] add_index :activity_streams, [:created_at] add_index :activity_streams, [:activity_stream_object_id] @@ -458,6 +460,7 @@ class CreateBase < ActiveRecord::Migration[4.2] t.timestamps limit: 3, null: false end add_index :stores, %i[store_object_id o_id] + add_index :stores, %i[store_file_id] add_foreign_key :stores, :store_objects add_foreign_key :stores, :store_files add_foreign_key :stores, :users, column: :created_by_id @@ -675,6 +678,7 @@ class CreateBase < ActiveRecord::Migration[4.2] add_index :cti_caller_ids, %i[caller_id level] add_index :cti_caller_ids, %i[caller_id user_id] add_index :cti_caller_ids, %i[object o_id] + add_index :cti_caller_ids, %i[object o_id level user_id caller_id], name: 'index_cti_caller_ids_on_object_o_id_level_user_id_caller_id' add_foreign_key :cti_caller_ids, :users create_table :stats_stores do |t| diff --git a/db/migrate/20120101000010_create_ticket.rb b/db/migrate/20120101000010_create_ticket.rb index 6604e2e13..f62877074 100644 --- a/db/migrate/20120101000010_create_ticket.rb +++ b/db/migrate/20120101000010_create_ticket.rb @@ -87,9 +87,11 @@ class CreateTicket < ActiveRecord::Migration[4.2] add_index :tickets, [:group_id] add_index :tickets, [:owner_id] add_index :tickets, [:customer_id] + add_index :tickets, %i[customer_id state_id created_at] add_index :tickets, [:number], unique: true add_index :tickets, [:title] add_index :tickets, [:created_at] + add_index :tickets, [:updated_at] add_index :tickets, [:first_response_at] add_index :tickets, [:first_response_escalation_at] add_index :tickets, [:first_response_in_min] @@ -113,6 +115,12 @@ class CreateTicket < ActiveRecord::Migration[4.2] add_index :tickets, [:time_unit] add_index :tickets, %i[group_id state_id] add_index :tickets, %i[group_id state_id owner_id] + add_index :tickets, %i[group_id state_id updated_at] + add_index :tickets, %i[group_id state_id owner_id updated_at], name: 'index_tickets_on_group_id_state_id_owner_id_updated_at' + add_index :tickets, %i[group_id state_id created_at] + add_index :tickets, %i[group_id state_id owner_id created_at], name: 'index_tickets_on_group_id_state_id_owner_id_created_at' + add_index :tickets, %i[group_id state_id close_at] + add_index :tickets, %i[group_id state_id owner_id close_at], name: 'index_tickets_on_group_id_state_id_owner_id_close_at' add_foreign_key :tickets, :groups add_foreign_key :tickets, :users, column: :owner_id add_foreign_key :tickets, :users, column: :customer_id diff --git a/db/migrate/20190405000001_database_indexes.rb b/db/migrate/20190405000001_database_indexes.rb new file mode 100644 index 000000000..ed212a3ed --- /dev/null +++ b/db/migrate/20190405000001_database_indexes.rb @@ -0,0 +1,72 @@ +class DatabaseIndexes < ActiveRecord::Migration[5.1] + def change + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + map = { + activity_streams: [ + { + columns: %i[permission_id group_id], + }, + { + columns: %i[permission_id group_id created_at], + name: 'index_activity_streams_on_permission_id_group_id_created_at', + }, + ], + stores: [ + { + columns: [:store_file_id], + }, + ], + cti_caller_ids: [ + { + columns: %i[object o_id level user_id caller_id], + name: 'index_cti_caller_ids_on_object_o_id_level_user_id_caller_id', + }, + ], + tickets: [ + { + columns: [:updated_at], + }, + { + columns: %i[customer_id state_id created_at], + }, + { + columns: %i[group_id state_id updated_at], + }, + { + columns: %i[group_id state_id owner_id updated_at], + name: 'index_tickets_on_group_id_state_id_owner_id_updated_at', + }, + { + columns: %i[group_id state_id created_at], + }, + { + columns: %i[group_id state_id owner_id created_at], + name: 'index_tickets_on_group_id_state_id_owner_id_created_at', + }, + { + columns: %i[group_id state_id close_at], + }, + { + columns: %i[group_id state_id owner_id close_at], + name: 'index_tickets_on_group_id_state_id_owner_id_close_at', + }, + ] + } + + map.each do |table, indexes| + indexes.each do |index| + + params = [table, index[:columns]] + params.push(name: index[:name]) if index[:name] + + next if index_exists?(*params) + + add_index(*params) + end + end + + end +end diff --git a/lib/session_helper.rb b/lib/session_helper.rb index 878af387e..d459e3426 100644 --- a/lib/session_helper.rb +++ b/lib/session_helper.rb @@ -40,7 +40,7 @@ module SessionHelper end def self.list(limit = 10_000) - ActiveRecord::SessionStore::Session.order('updated_at DESC').limit(limit) + ActiveRecord::SessionStore::Session.order(updated_at: :desc).limit(limit) end def self.destroy(id) diff --git a/lib/sessions/event/chat_session_start.rb b/lib/sessions/event/chat_session_start.rb index 9614a4e4a..f38c80c8f 100644 --- a/lib/sessions/event/chat_session_start.rb +++ b/lib/sessions/event/chat_session_start.rb @@ -5,7 +5,7 @@ class Sessions::Event::ChatSessionStart < Sessions::Event::ChatBase return if !permission_check('chat.agent', 'chat') # find first in waiting list - chat_session = Chat::Session.where(state: 'waiting').order('created_at ASC').first + chat_session = Chat::Session.where(state: 'waiting').order(created_at: :asc).first if !chat_session return { event: 'chat_session_start', diff --git a/lib/sessions/event/chat_status_agent.rb b/lib/sessions/event/chat_status_agent.rb index 82af10669..657a0d704 100644 --- a/lib/sessions/event/chat_status_agent.rb +++ b/lib/sessions/event/chat_status_agent.rb @@ -11,7 +11,7 @@ class Sessions::Event::ChatStatusAgent < Sessions::Event::ChatBase Chat::Agent.state(@session['id'], state) # update recipients of existing sessions - Chat::Session.where(state: 'running', user_id: @session['id']).order('created_at ASC').each do |chat_session| + Chat::Session.where(state: 'running', user_id: @session['id']).order(created_at: :asc).each do |chat_session| chat_session.add_recipient(@client_id, true) end { diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb index 42629063d..af8b5a163 100644 --- a/spec/models/channel/email_parser_spec.rb +++ b/spec/models/channel/email_parser_spec.rb @@ -349,7 +349,7 @@ RSpec.describe Channel::EmailParser, type: :model do # see https://github.com/zammad/zammad/issues/2486 context 'when image is large but not resizable' do let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail079.box') } - let(:attachment) { article.attachments.last } + let(:attachment) { article.attachments.to_a.find { |i| i.filename == 'a.jpg' } } let(:article) { described_class.new.process({}, raw_mail).second } it "doesn't set resizable preference" do diff --git a/spec/models/cti/log_spec.rb b/spec/models/cti/log_spec.rb index 45a1eae14..e508790d3 100644 --- a/spec/models/cti/log_spec.rb +++ b/spec/models/cti/log_spec.rb @@ -9,7 +9,12 @@ RSpec.describe Cti::Log do end context 'when over 60 Log records exist' do - subject!(:cti_logs) { create_list(:'cti/log', 61) } + subject!(:cti_logs) do + 61.times.map do |_i| # rubocop:disable Performance/TimesMap + travel 1.second + create(:'cti/log') + end + end it 'returns the 60 latest ones in the :list key' do expect(Cti::Log.log[:list]).to match_array(cti_logs.last(60)) diff --git a/spec/requests/integration/cti_spec.rb b/spec/requests/integration/cti_spec.rb index f8c12f234..37f21945f 100644 --- a/spec/requests/integration/cti_spec.rb +++ b/spec/requests/integration/cti_spec.rb @@ -252,6 +252,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_between(2, 3) expect(log.duration_talking_time).to be_between(2, 3) + travel 1.second + # inbound - I - new call params = 'event=newCall&direction=in&to=4930600000000&from=4912347114711&call_id=1234567890-3&user%5B%5D=user+1' post "/api/v1/cti/#{token}", params: params @@ -273,6 +275,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - I - answer by customer params = 'event=answer&direction=in&call_id=1234567890-3&to=4930600000000&from=4912347114711' post "/api/v1/cti/#{token}", params: params @@ -294,6 +298,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - I - hangup by customer params = 'event=hangup&direction=in&call_id=1234567890-3&cause=normalClearing&to=4930600000000&from=4912347114711' post "/api/v1/cti/#{token}", params: params @@ -315,6 +321,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_truthy + travel 1.second + # inbound - II - new call params = 'event=newCall&direction=in&to=4930600000000&from=4912347114711&call_id=1234567890-4&user%5B%5D=user+1,user+2' post "/api/v1/cti/#{token}", params: params @@ -336,6 +344,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - II - answer by voicemail params = 'event=answer&direction=in&call_id=1234567890-4&to=4930600000000&from=4912347114711&user=voicemail' post "/api/v1/cti/#{token}", params: params @@ -357,6 +367,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - II - hangup by customer params = 'event=hangup&direction=in&call_id=1234567890-4&cause=normalClearing&to=4930600000000&from=4912347114711' post "/api/v1/cti/#{token}", params: params @@ -378,6 +390,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_truthy + travel 1.second + # inbound - III - new call params = 'event=newCall&direction=in&to=4930600000000&from=4912347114711&call_id=1234567890-5&user%5B%5D=user+1,user+2' post "/api/v1/cti/#{token}", params: params @@ -399,6 +413,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - III - hangup by customer params = 'event=hangup&direction=in&call_id=1234567890-5&cause=normalClearing&to=4930600000000&from=4912347114711' post "/api/v1/cti/#{token}", params: params @@ -420,6 +436,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - IV - new call params = 'event=newCall&direction=in&to=4930600000000&from=49999992222222&call_id=1234567890-6&user%5B%5D=user+1,user+2' post "/api/v1/cti/#{token}", params: params @@ -443,6 +461,8 @@ RSpec.describe 'Integration CTI', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - IV - new call params = 'event=newCall&direction=in&to=4930600000000&from=anonymous&call_id=1234567890-7&user%5B%5D=user+1,user+2&queue=some_queue_name' post "/api/v1/cti/#{token}", params: params diff --git a/spec/requests/integration/placetel_spec.rb b/spec/requests/integration/placetel_spec.rb index e7f531402..0d010a345 100644 --- a/spec/requests/integration/placetel_spec.rb +++ b/spec/requests/integration/placetel_spec.rb @@ -198,6 +198,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # outbound - I - hangup by agent params = 'event=HungUp&call_id=1234567890-1&type=missed' post "/api/v1/placetel/#{token}", params: params @@ -218,6 +220,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # outbound - II - new call params = 'event=newCall&direction=out&from=030600000000&to=01114100300&call_id=1234567890-2' post "/api/v1/placetel/#{token}", params: params @@ -238,6 +242,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # outbound - II - answer by customer params = 'event=CallAccepted&call_id=1234567890-2&from=030600000000&to=01114100300' post "/api/v1/placetel/#{token}", params: params @@ -258,6 +264,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # outbound - II - hangup by customer params = 'event=HungUp&call_id=1234567890-2&type=accepted&from=030600000000&to=01114100300' post "/api/v1/placetel/#{token}", params: params @@ -278,6 +286,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_truthy + travel 1.second + # inbound - I - new call params = 'event=IncomingCall&to=030600000000&from=01114100300&call_id=1234567890-3' post "/api/v1/placetel/#{token}", params: params @@ -298,6 +308,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - I - answer by customer params = 'event=CallAccepted&call_id=1234567890-3&to=030600000000&from=01114100300' post "/api/v1/placetel/#{token}", params: params @@ -318,6 +330,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - I - hangup by customer params = 'event=HungUp&call_id=1234567890-3&type=accepted&to=030600000000&from=01114100300' post "/api/v1/placetel/#{token}", params: params @@ -338,6 +352,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_truthy + travel 1.second + # inbound - II - new call params = 'event=IncomingCall&to=030600000000&from=01114100300&call_id=1234567890-4' post "/api/v1/placetel/#{token}", params: params @@ -358,6 +374,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - II - answer by voicemail params = 'event=CallAccepted&call_id=1234567890-4&to=030600000000&from=01114100300&user=voicemail' post "/api/v1/placetel/#{token}", params: params @@ -378,6 +396,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - II - hangup by customer params = 'event=HungUp&call_id=1234567890-4&type=accepted&to=030600000000&from=01114100300' post "/api/v1/placetel/#{token}", params: params @@ -398,6 +418,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_truthy + travel 1.second + # inbound - III - new call params = 'event=IncomingCall&to=030600000000&from=01114100300&call_id=1234567890-5' post "/api/v1/placetel/#{token}", params: params @@ -418,6 +440,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - III - hangup by customer params = 'event=HungUp&call_id=1234567890-5&type=accepted&to=030600000000&from=01114100300' post "/api/v1/placetel/#{token}", params: params @@ -438,6 +462,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_truthy expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - IV - new call params = 'event=IncomingCall&to=030600000000&from=49999992222222&call_id=1234567890-6' post "/api/v1/placetel/#{token}", params: params @@ -460,6 +486,8 @@ RSpec.describe 'Integration Placetel', type: :request do expect(log.duration_waiting_time).to be_nil expect(log.duration_talking_time).to be_nil + travel 1.second + # inbound - IV - new call params = 'event=IncomingCall&to=030600000000&from=anonymous&call_id=1234567890-7' post "/api/v1/placetel/#{token}", params: params diff --git a/spec/requests/integration/sipgate_spec.rb b/spec/requests/integration/sipgate_spec.rb index 472a55270..4975a4980 100644 --- a/spec/requests/integration/sipgate_spec.rb +++ b/spec/requests/integration/sipgate_spec.rb @@ -217,6 +217,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('newCall') expect(log.done).to eq(true) + travel 1.second + # outbound - I - hangup by agent params = 'event=hangup&direction=out&callId=1234567890-1&cause=cancel' post '/api/v1/sipgate/out', params: params @@ -232,6 +234,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('hangup') expect(log.done).to eq(true) + travel 1.second + # outbound - II - new call params = 'event=newCall&direction=out&from=4930600000000&to=4912347114711&callId=1234567890-2&user%5B%5D=user+1' post '/api/v1/sipgate/out', params: params @@ -247,6 +251,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('newCall') expect(log.done).to eq(true) + travel 1.second + # outbound - II - answer by customer params = 'event=answer&direction=out&callId=1234567890-2&from=4930600000000&to=4912347114711' post '/api/v1/sipgate/out', params: params @@ -262,6 +268,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('answer') expect(log.done).to eq(true) + travel 1.second + # outbound - II - hangup by customer params = 'event=hangup&direction=out&callId=1234567890-2&cause=normalClearing&from=4930600000000&to=4912347114711' post '/api/v1/sipgate/out', params: params @@ -277,6 +285,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('hangup') expect(log.done).to eq(true) + travel 1.second + # inbound - I - new call params = 'event=newCall&direction=in&to=4930600000000&from=4912347114711&callId=1234567890-3&user%5B%5D=user+1' post '/api/v1/sipgate/in', params: params @@ -292,6 +302,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('newCall') expect(log.done).to eq(false) + travel 1.second + # inbound - I - answer by customer params = 'event=answer&direction=in&callId=1234567890-3&to=4930600000000&from=4912347114711' post '/api/v1/sipgate/in', params: params @@ -307,6 +319,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('answer') expect(log.done).to eq(true) + travel 1.second + # inbound - I - hangup by customer params = 'event=hangup&direction=in&callId=1234567890-3&cause=normalClearing&to=4930600000000&from=4912347114711' post '/api/v1/sipgate/in', params: params @@ -322,6 +336,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('hangup') expect(log.done).to eq(true) + travel 1.second + # inbound - II - new call params = 'event=newCall&direction=in&to=4930600000000&from=4912347114711&callId=1234567890-4&user%5B%5D=user+1,user+2' post '/api/v1/sipgate/in', params: params @@ -337,6 +353,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('newCall') expect(log.done).to eq(false) + travel 1.second + # inbound - II - answer by voicemail params = 'event=answer&direction=in&callId=1234567890-4&to=4930600000000&from=4912347114711&user=voicemail' post '/api/v1/sipgate/in', params: params @@ -352,6 +370,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('answer') expect(log.done).to eq(true) + travel 1.second + # inbound - II - hangup by customer params = 'event=hangup&direction=in&callId=1234567890-4&cause=normalClearing&to=4930600000000&from=4912347114711' post '/api/v1/sipgate/in', params: params @@ -367,6 +387,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('hangup') expect(log.done).to eq(false) + travel 1.second + # inbound - III - new call params = 'event=newCall&direction=in&to=4930600000000&from=4912347114711&callId=1234567890-5&user%5B%5D=user+1,user+2' post '/api/v1/sipgate/in', params: params @@ -382,6 +404,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('newCall') expect(log.done).to eq(false) + travel 1.second + # inbound - III - hangup by customer params = 'event=hangup&direction=in&callId=1234567890-5&cause=normalClearing&to=4930600000000&from=4912347114711' post '/api/v1/sipgate/in', params: params @@ -397,6 +421,8 @@ RSpec.describe 'Integration Sipgate', type: :request do expect(log.state).to eq('hangup') expect(log.done).to eq(false) + travel 1.second + # inbound - IV - new call params = 'event=newCall&direction=in&to=4930600000000&from=49999992222222&callId=1234567890-6&user%5B%5D=user+1,user+2' post '/api/v1/sipgate/in', params: params diff --git a/test/unit/karma_test.rb b/test/unit/karma_test.rb index 87634351b..2f1475459 100644 --- a/test/unit/karma_test.rb +++ b/test/unit/karma_test.rb @@ -6,7 +6,7 @@ class KarmaTest < ActiveSupport::TestCase groups = Group.all roles = Role.where(name: 'Agent') - agent1 = User.create_or_update( + agent1 = User.create!( login: 'karma-agent1@example.com', firstname: 'Karma', lastname: 'Agent1', @@ -18,7 +18,7 @@ class KarmaTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - agent2 = User.create_or_update( + agent2 = User.create!( login: 'karma-agent2@example.com', firstname: 'Karma', lastname: 'Agent2', @@ -30,7 +30,7 @@ class KarmaTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - customer1 = User.create_or_update( + customer1 = User.create!( login: 'karma-customer1@example.com', firstname: 'Karma', lastname: 'Customer1', @@ -42,7 +42,7 @@ class KarmaTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket1 = Ticket.create( + ticket1 = Ticket.create!( title: 'karma test 1', group: Group.lookup(name: 'Users'), customer: customer1, @@ -53,7 +53,7 @@ class KarmaTest < ActiveSupport::TestCase updated_at: Time.zone.now - 10.hours, created_at: Time.zone.now - 10.hours, ) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -139,7 +139,7 @@ class KarmaTest < ActiveSupport::TestCase ticket1.state = Ticket::State.lookup(name: 'open') ticket1.save! - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -178,7 +178,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -202,7 +202,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -226,7 +226,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -250,7 +250,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -274,7 +274,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -298,7 +298,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5 + 10, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -345,7 +345,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5 + 10 + 4, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - ticket2 = Ticket.create( + ticket2 = Ticket.create!( title: 'karma test 1', group: Group.lookup(name: 'Users'), customer: customer1, @@ -357,7 +357,7 @@ class KarmaTest < ActiveSupport::TestCase updated_at: Time.zone.now - 10.hours, created_at: Time.zone.now - 10.hours, ) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket2.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -408,7 +408,7 @@ class KarmaTest < ActiveSupport::TestCase assert_equal(5 + 10 + 4, Karma.score_by_user(agent2)) assert_equal(0, Karma.score_by_user(customer1)) - calendar1 = Calendar.create_or_update( + calendar1 = Calendar.create!( name: 'EU 1 - karma test', timezone: 'Europe/Berlin', business_hours: { @@ -447,7 +447,7 @@ class KarmaTest < ActiveSupport::TestCase created_by_id: 1, ) - sla1 = Sla.create_or_update( + sla1 = Sla.create!( name: 'test sla 1', condition: {}, first_response_time: 20,