Performance: Observing every Overview of each Agent Session doesn't scale well on larger systems (60 Overviews per Agent). With this change only the last 5 used Overviews are checked on every iteration. A full check is still performed ever 60 seconds. This reduces the overall load.

This commit is contained in:
Martin Edenhofer 2019-11-28 07:59:22 +01:00 committed by Thorsten Eckel
parent f41aa097ce
commit 4e8cf209ca
12 changed files with 215 additions and 48 deletions

View file

@ -936,6 +936,10 @@ class Navbar extends App.Controller
if item.link is @view if item.link is @view
@title item.name, true @title item.name, true
# send first view info
if !@view && data && data[0] && data[0].link
App.WebSocket.send(event:'ticket_overview_select', data: { view: data[0].link })
# redirect to first view # redirect to first view
if @activeState && !@view && !@vertical if @activeState && !@view && !@vertical
view = data[0].link view = data[0].link
@ -1037,6 +1041,8 @@ class Table extends App.Controller
@view_mode = App.LocalStorage.get("mode:#{@view}", @Session.get('id')) || 's' @view_mode = App.LocalStorage.get("mode:#{@view}", @Session.get('id')) || 's'
console.log 'notice', 'view:', @view, @view_mode console.log 'notice', 'view:', @view, @view_mode
App.WebSocket.send(event:'ticket_overview_select', data: { view: @view })
# get ticket list # get ticket list
ticketListShow = [] ticketListShow = []
for ticket in tickets for ticket in tickets

View file

@ -894,6 +894,10 @@ class App.TicketZoom extends App.Controller
if macro && macro.ux_flow_next_up if macro && macro.ux_flow_next_up
taskAction = macro.ux_flow_next_up taskAction = macro.ux_flow_next_up
nextTicket = undefined
if taskAction is 'closeNextInOverview' || taskAction is 'next_from_overview'
nextTicket = @getNextTicketInOverview()
# submit changes # submit changes
@ajax( @ajax(
id: "ticket_update_#{ticket.id}" id: "ticket_update_#{ticket.id}"
@ -916,8 +920,8 @@ class App.TicketZoom extends App.Controller
@sidebarWidget.commit() @sidebarWidget.commit()
if taskAction is 'closeNextInOverview' || taskAction is 'next_from_overview' if taskAction is 'closeNextInOverview' || taskAction is 'next_from_overview'
@openTicketInOverview(nextTicket)
App.Event.trigger('overview:fetch') App.Event.trigger('overview:fetch')
@taskOpenNextTicketInOverview()
return return
if taskAction is 'closeTab' || taskAction is 'next_task' if taskAction is 'closeTab' || taskAction is 'next_task'

View file

@ -22,22 +22,40 @@ App.TicketNavigable =
show: true show: true
) )
getNextTicketInOverview: ->
return if !@ticket
return if !@overview_id
App.Overview.find(@overview_id).nextTicket(@ticket)
openTicketInOverview: (nextTicket) ->
if nextTicket
@taskCloseTicket()
@taskLoadTicket(nextTicket.id)
return
@taskCloseTicket(true)
taskOpenNextTicketInOverview: -> taskOpenNextTicketInOverview: ->
if !(@overview_id? && @ticket?) if !(@overview_id? && @ticket?)
@taskCloseTicket(true) @taskCloseTicket(true)
return return
next_ticket = App.Overview.find(@overview_id).nextTicket(@ticket)
if next_ticket nextTicket = @getNextTicketInOverview()
if nextTicket
@taskCloseTicket() @taskCloseTicket()
@taskLoadTicket(next_ticket.id) @taskLoadTicket(nextTicket.id)
return return
@taskCloseTicket(true) @taskCloseTicket(true)
taskCloseTicket: (openNext = false) -> taskCloseTicket: (openNext = false) ->
App.TaskManager.remove(@taskKey) App.TaskManager.remove(@taskKey)
return if !openNext return if !openNext
nextTaskUrl = App.TaskManager.nextTaskUrl() nextTaskUrl = App.TaskManager.nextTaskUrl()
if nextTaskUrl if nextTaskUrl
@navigate nextTaskUrl @navigate nextTaskUrl
return return
@navigate '#' @navigate '#'

View file

@ -7,6 +7,10 @@ all overviews by user
result = Ticket::Overviews.all(current_user: User.find(3)) result = Ticket::Overviews.all(current_user: User.find(3))
certain overviews by user
result = Ticket::Overviews.all(current_user: User.find(3), links: ['all_unassigned', 'my_assigned'])
returns returns
result = [overview1, overview2] result = [overview1, overview2]
@ -15,6 +19,7 @@ returns
def self.all(data) def self.all(data)
current_user = data[:current_user] current_user = data[:current_user]
links = data[:links]
# get customer overviews # get customer overviews
role_ids = User.joins(:roles).where(users: { id: current_user.id, active: true }, roles: { active: true }).pluck('roles.id') role_ids = User.joins(:roles).where(users: { id: current_user.id, active: true }, roles: { active: true }).pluck('roles.id')
@ -23,6 +28,9 @@ returns
if current_user.organization_id && current_user.organization.shared if current_user.organization_id && current_user.organization.shared
overview_filter.delete(:organization_shared) overview_filter.delete(:organization_shared)
end end
if links.present?
overview_filter[:link] = links
end
overviews = Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).distinct('overview.id').order(:prio, :name) overviews = Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).distinct('overview.id').order(:prio, :name)
return overviews return overviews
end end
@ -35,12 +43,21 @@ returns
if User.where('out_of_office = ? AND out_of_office_start_at <= ? AND out_of_office_end_at >= ? AND out_of_office_replacement_id = ? AND active = ?', true, Time.zone.today, Time.zone.today, current_user.id, true).count.positive? if User.where('out_of_office = ? AND out_of_office_start_at <= ? AND out_of_office_end_at >= ? AND out_of_office_replacement_id = ? AND active = ?', true, Time.zone.today, Time.zone.today, current_user.id, true).count.positive?
overview_filter_not = {} overview_filter_not = {}
end end
if links.present?
overview_filter[:link] = links
end
Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).where.not(overview_filter_not).distinct('overview.id').order(:prio, :name) Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).where.not(overview_filter_not).distinct('overview.id').order(:prio, :name)
end end
=begin =begin
result = Ticket::Overviews.index(User.find(123)) index of all overviews by user
result = Ticket::Overviews.index(User.find(3))
index of certain overviews by user
result = Ticket::Overviews.index(User.find(3), ['all_unassigned', 'my_assigned'])
returns returns
@ -75,9 +92,10 @@ returns
=end =end
def self.index(user) def self.index(user, links = nil)
overviews = Ticket::Overviews.all( overviews = Ticket::Overviews.all(
current_user: user, current_user: user,
links: links,
) )
return [] if overviews.blank? return [] if overviews.blank?

View file

@ -12,21 +12,17 @@ Rails.application.configure do
# Show full error reports. # Show full error reports.
config.consider_all_requests_local = true config.consider_all_requests_local = true
# Commented out to ensure using file cache store as described in config/application.rb
# Enable/disable caching. By default caching is disabled. # Enable/disable caching. By default caching is disabled.
# Run rails dev:cache to toggle caching. # Run rails dev:cache to toggle caching.
# if Rails.root.join('tmp', 'caching-dev.txt').exist? if Rails.root.join('tmp', 'caching-dev.txt').exist?
# config.action_controller.perform_caching = true config.action_controller.perform_caching = true
#
# config.cache_store = :memory_store config.public_file_server.headers = {
# config.public_file_server.headers = { 'Cache-Control' => "public, max-age=#{2.days.to_i}"
# 'Cache-Control' => "public, max-age=#{2.days.to_i}" }
# } else
# else config.action_controller.perform_caching = false
# config.action_controller.perform_caching = false end
#
# config.cache_store = :null_store
# end
# Store uploaded files on the local file system (see config/storage.yml for options) # Store uploaded files on the local file system (see config/storage.yml for options)
# config.active_storage.service = :local # config.active_storage.service = :local

View file

@ -4,31 +4,82 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
Cache.write("TicketOverviewPull::#{user_id}", { needed: true }) Cache.write("TicketOverviewPull::#{user_id}", { needed: true })
end end
def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 8) def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 7)
@user = user @user = user
@client = client @client = client
@client_id = client_id @client_id = client_id
@ttl = ttl @ttl = ttl
@asset_lookup = asset_lookup @asset_lookup = asset_lookup
@last_change = nil @last_index_lists = nil
@last_overview = {} @last_overview = {}
@last_overview_change = nil @last_overview_change = nil
@last_ticket_change = nil @last_ticket_change = nil
@last_full_fetch = nil
end
def self.overview_history_append(overview, user_id)
key = "TicketOverviewHistory::#{user_id}"
history = Cache.get(key) || []
history.prepend overview
history.uniq!
if history.count > 4
history.pop
end
Cache.write(key, history)
end
def self.overview_history_get(user_id)
Cache.get("TicketOverviewHistory::#{user_id}")
end end
def load def load
# get whole collection # get whole collection
index_and_lists = Ticket::Overviews.index(@user) index_and_lists = nil
local_overview_changed = overview_changed?
if !@last_index_lists || !@last_full_fetch || @last_full_fetch < (Time.zone.now.to_i - 60) || local_overview_changed
# check if min one ticket has changed
return if !ticket_changed?(true) && !local_overview_changed
index_and_lists = Ticket::Overviews.index(@user)
@last_full_fetch = Time.zone.now.to_i
else
# check if min one ticket has changed
return if !ticket_changed? && !local_overview_changed
index_and_lists_local = Ticket::Overviews.index(@user, Sessions::Backend::TicketOverviewList.overview_history_get(@user.id))
# compare index_and_lists_local to index_and_lists_local
# return if no changes
index_and_lists = []
@last_index_lists.each do |last_index|
found_in_particular_index = false
index_and_lists_local.each do |local_index|
next if local_index[:overview][:id] != last_index[:overview][:id]
index_and_lists.push local_index
found_in_particular_index = true
break
end
next if found_in_particular_index == true
index_and_lists.push last_index
end
end
# no data exists # no data exists
return if index_and_lists.blank? return if index_and_lists.blank?
# no change exists # no change exists
return if @last_change == index_and_lists return if @last_index_lists == index_and_lists
# remember last state # remember last state
@last_change = index_and_lists @last_index_lists = index_and_lists
index_and_lists index_and_lists
end end
@ -36,6 +87,12 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
def local_to_run? def local_to_run?
return false if !@time_now return false if !@time_now
return true if pull_overview?
false
end
def pull_overview?
result = Cache.get("TicketOverviewPull::#{@user.id}") result = Cache.get("TicketOverviewPull::#{@user.id}")
Cache.delete("TicketOverviewPull::#{@user.id}") if result Cache.delete("TicketOverviewPull::#{@user.id}") if result
return true if result return true if result
@ -48,14 +105,6 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
@time_now = Time.zone.now.to_i @time_now = Time.zone.now.to_i
# check if min one ticket or overview has changed
last_overview_change = Overview.latest_change
last_ticket_change = Ticket.latest_change
return if last_ticket_change == @last_ticket_change && last_overview_change == @last_overview_change
@last_overview_change = last_overview_change
@last_ticket_change = last_ticket_change
# load current data # load current data
index_and_lists = load index_and_lists = load
return if !index_and_lists return if !index_and_lists
@ -134,4 +183,26 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
nil nil
end end
def overview_changed?
# check if min one overview has changed
last_overview_change = Overview.latest_change
return false if last_overview_change == @last_overview_change
@last_overview_change = last_overview_change
true
end
def ticket_changed?(reset = false)
# check if min one ticket has changed
last_ticket_change = Ticket.latest_change
return false if last_ticket_change == @last_ticket_change
@last_ticket_change = last_ticket_change if reset
true
end
end end

View file

@ -0,0 +1,23 @@
class Sessions::Event::TicketOverviewSelect < Sessions::Event::Base
=begin
Event module to serve spool messages and send them to new client connection.
To execute this manually, just paste the following into the browser console
App.WebSocket.send({event:'spool'})
=end
def run
return if @payload['data'].blank?
return if @payload['data']['view'].blank?
return if @session['id'].blank?
Sessions::Backend::TicketOverviewList.overview_history_append(@payload['data']['view'], @session['id'])
nil
end
end

View file

@ -82,14 +82,14 @@ RSpec.describe Sessions::Backend::TicketOverviewList do
let!(:first_call) { collection.push } let!(:first_call) { collection.push }
it 'returns nil' do it 'returns nil' do
expect(collection.push).to be(nil) expect(collection.push).to eq(nil)
end end
context 'even after the TTL has passed' do context 'even after the TTL has passed' do
before { travel(ttl + 1) } before { travel(ttl + 1) }
it 'returns nil' do it 'returns nil' do
expect(collection.push).to be(nil) expect(collection.push).to eq(nil)
end end
end end
@ -97,7 +97,7 @@ RSpec.describe Sessions::Backend::TicketOverviewList do
before { described_class.reset(admin.id) } before { described_class.reset(admin.id) }
it 'returns nil' do it 'returns nil' do
expect(collection.push).to be(nil) expect(collection.push).to eq(nil)
end end
end end
end end
@ -108,14 +108,14 @@ RSpec.describe Sessions::Backend::TicketOverviewList do
context 'before the TTL has passed' do context 'before the TTL has passed' do
it 'returns nil' do it 'returns nil' do
expect(collection.push).to be(nil) expect(collection.push).to eq(nil)
end end
context 'after .reset with the users id' do context 'after .reset with the users id' do
before { described_class.reset(admin.id) } before { described_class.reset(admin.id) }
it 'returns nil because no ticket and no overview has changed' do it 'returns nil because no ticket and no overview has changed' do
expect(collection.push).to be nil expect(collection.push).to eq(nil)
end end
end end
end end
@ -124,7 +124,7 @@ RSpec.describe Sessions::Backend::TicketOverviewList do
before { travel(ttl + 1) } before { travel(ttl + 1) }
it 'returns an empty result' do it 'returns an empty result' do
expect(collection.push).to eq nil expect(collection.push).to eq(nil)
end end
end end
@ -132,7 +132,7 @@ RSpec.describe Sessions::Backend::TicketOverviewList do
before { travel(2.hours + 1.second) } before { travel(2.hours + 1.second) }
it 'returns an empty result' do it 'returns an empty result' do
expect(collection.push).to eq nil expect(collection.push).to eq(nil)
end end
end end
end end

View file

@ -101,6 +101,35 @@ class AgentTicketLinkTest < TestCase
css: '.content.active .ticketLinks', css: '.content.active .ticketLinks',
value: ticket2[:title], value: ticket2[:title],
) )
# cleanup
ticket_open_by_search(
browser: browser2,
number: ticket1[:number],
)
sleep 1
ticket_update(
browser: browser2,
data: {
state: 'closed',
}
)
tasks_close_all()
ticket_open_by_search(
browser: browser2,
number: ticket2[:number],
)
sleep 1
ticket_update(
browser: browser2,
data: {
state: 'closed',
}
)
end end
end end

View file

@ -15,7 +15,7 @@ class AgentTicketMacroTest < TestCase
data: { data: {
customer: 'nico', customer: 'nico',
group: 'Users', group: 'Users',
title: 'some subject - macro "Close & Tag as Spam" default', title: 'macro "Close & Tag as Spam" default',
body: 'some body - macro "Close & Tag as Spam" default', body: 'some body - macro "Close & Tag as Spam" default',
}, },
) )
@ -66,7 +66,7 @@ class AgentTicketMacroTest < TestCase
data: { data: {
customer: 'nico', customer: 'nico',
group: 'Users', group: 'Users',
title: "some subject - macro #{macro_name}", title: "macro #{macro_name}",
body: "some body - macro #{macro_name}", body: "some body - macro #{macro_name}",
}, },
) )
@ -105,7 +105,7 @@ class AgentTicketMacroTest < TestCase
data: { data: {
customer: 'nico', customer: 'nico',
group: 'Users', group: 'Users',
title: "some subject - macro #{macro_name}", title: "macro #{macro_name}",
body: "some body - macro #{macro_name}", body: "some body - macro #{macro_name}",
}, },
) )
@ -134,7 +134,7 @@ class AgentTicketMacroTest < TestCase
ux_flow_next_up: ux_flow_next_up, ux_flow_next_up: ux_flow_next_up,
) )
title_prefix = "some subject - macro #{macro_name}" title_prefix = "macro #{macro_name}"
ticket1 = ticket_create( ticket1 = ticket_create(
data: { data: {
customer: 'nico', customer: 'nico',

View file

@ -45,7 +45,6 @@ class AgentTicketOverviewTabTest < TestCase
# js: '$(".content.active .sidebar").css("display", "block")', # js: '$(".content.active .sidebar").css("display", "block")',
#) #)
#click(text: 'Unassigned & Open') #click(text: 'Unassigned & Open')
sleep 8 # till overview is rendered
ticket_open_by_overview( ticket_open_by_overview(
number: ticket1[:number], number: ticket1[:number],
@ -63,9 +62,10 @@ class AgentTicketOverviewTabTest < TestCase
task_type: 'closeNextInOverview', # default: stayOnTab / possible: closeTab, closeNextInOverview, stayOnTab task_type: 'closeNextInOverview', # default: stayOnTab / possible: closeTab, closeNextInOverview, stayOnTab
) )
match( watch_for(
css: '.tasks .task.is-active', css: '.tasks .task.is-active',
value: "overview tab test #2 - #{title}", value: "overview tab test #2 - #{title}",
timeout: 8,
) )
assert_equal(1, @browser.find_elements(css: '.tasks .task').count) assert_equal(1, @browser.find_elements(css: '.tasks .task').count)

View file

@ -2721,6 +2721,8 @@ wait untill text in selector disabppears
end end
else else
6.times do 6.times do
# prefere find_elements ofer find_element because of exception handling
element = instance.find_elements(partial_link_text: params[:number])[0] element = instance.find_elements(partial_link_text: params[:number])[0]
break if element break if element
@ -2733,7 +2735,7 @@ wait untill text in selector disabppears
end end
element.click element.click
sleep 1 sleep 1
number = instance.find_elements(css: '.content.active .ticketZoom-header .ticket-number')[0].text number = instance.find_element(css: '.content.active .ticketZoom-header .ticket-number').text
if !number.match?(/#{params[:number]}/) if !number.match?(/#{params[:number]}/)
screenshot(browser: instance, comment: 'ticket_open_by_overview_open_failed_failed') screenshot(browser: instance, comment: 'ticket_open_by_overview_open_failed_failed')
raise "unable to open ticket #{params[:number]}!" raise "unable to open ticket #{params[:number]}!"