Refactoring:

- Changed behavior to use symbol as parameter to reduce memory coast.
- Removed code duplication.
- Added tests.
This commit is contained in:
Thorsten Eckel 2017-03-27 16:06:18 +02:00
parent f6c449327a
commit 57e7b9e987
11 changed files with 73 additions and 64 deletions

View file

@ -274,7 +274,7 @@ class TicketsController < ApplicationController
ticket_lists = Ticket ticket_lists = Ticket
.where( .where(
customer_id: ticket.customer_id, customer_id: ticket.customer_id,
state_id: Ticket::State.by_category('open') state_id: Ticket::State.by_category(:open)
) )
.where(access_condition) .where(access_condition)
.where('id != ?', [ ticket.id ]) .where('id != ?', [ ticket.id ])
@ -287,7 +287,7 @@ class TicketsController < ApplicationController
.where( .where(
customer_id: ticket.customer_id, customer_id: ticket.customer_id,
).where.not( ).where.not(
state_id: Ticket::State.by_category('merged') state_id: Ticket::State.by_category(:merged)
) )
.where(access_condition) .where(access_condition)
.where('id != ?', [ ticket.id ]) .where('id != ?', [ ticket.id ])
@ -503,7 +503,7 @@ class TicketsController < ApplicationController
condition = { condition = {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('open').map(&:id), value: Ticket::State.by_category(:open).pluck(:id),
}, },
'ticket.customer_id' => { 'ticket.customer_id' => {
operator: 'is', operator: 'is',
@ -521,7 +521,7 @@ class TicketsController < ApplicationController
condition = { condition = {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('closed').map(&:id), value: Ticket::State.by_category(:closed).pluck(:id),
}, },
'ticket.customer_id' => { 'ticket.customer_id' => {
operator: 'is', operator: 'is',
@ -577,7 +577,7 @@ class TicketsController < ApplicationController
condition = { condition = {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('open').map(&:id), value: Ticket::State.by_category(:open).pluck(:id),
}, },
'ticket.organization_id' => { 'ticket.organization_id' => {
operator: 'is', operator: 'is',
@ -595,7 +595,7 @@ class TicketsController < ApplicationController
condition = { condition = {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('closed').map(&:id), value: Ticket::State.by_category(:closed).pluck(:id),
}, },
'ticket.organization_id' => { 'ticket.organization_id' => {
operator: 'is', operator: 'is',

View file

@ -46,7 +46,7 @@ class Channel::Filter::MonitoringBase
customer = User.lookup(id: session_user_id) customer = User.lookup(id: session_user_id)
# follow up detection by meta data # follow up detection by meta data
open_states = Ticket::State.by_category('open') open_states = Ticket::State.by_category(:open)
Ticket.where(state: open_states).each { |ticket| Ticket.where(state: open_states).each { |ticket|
next if !ticket.preferences next if !ticket.preferences
next if !ticket.preferences['integration'] next if !ticket.preferences['integration']

View file

@ -7,14 +7,14 @@ class Observer::Ticket::UserTicketCounter::BackgroundJob
def perform def perform
# open ticket count # open ticket count
state_open = Ticket::State.by_category('open') state_open = Ticket::State.by_category(:open)
tickets_open = Ticket.where( tickets_open = Ticket.where(
customer_id: @customer_id, customer_id: @customer_id,
state_id: state_open, state_id: state_open,
).count() ).count()
# closed ticket count # closed ticket count
state_closed = Ticket::State.by_category('closed') state_closed = Ticket::State.by_category(:closed)
tickets_closed = Ticket.where( tickets_closed = Ticket.where(
customer_id: @customer_id, customer_id: @customer_id,
state_id: state_closed, state_id: state_closed,

View file

@ -14,7 +14,7 @@ returns
=end =end
def self.rebuild_all def self.rebuild_all
state_list_open = Ticket::State.by_category('open') state_list_open = Ticket::State.by_category(:open)
ticket_ids = Ticket.where(state_id: state_list_open).pluck(:id) ticket_ids = Ticket.where(state_id: state_list_open).pluck(:id)
ticket_ids.each { |ticket_id| ticket_ids.each { |ticket_id|

View file

@ -126,8 +126,8 @@ returns
def self.list_by_customer(data) def self.list_by_customer(data)
# get closed/open states # get closed/open states
state_list_open = Ticket::State.by_category( 'open' ) state_list_open = Ticket::State.by_category(:open)
state_list_closed = Ticket::State.by_category( 'closed' ) state_list_closed = Ticket::State.by_category(:closed)
# get tickets # get tickets
tickets_open = Ticket.where( tickets_open = Ticket.where(

View file

@ -14,51 +14,42 @@ class Ticket::State < ApplicationModel
=begin =begin
list tickets by customer looks up states for a given category
states = Ticket::State.by_category('open') # open|closed|work_on|work_on_all|viewable|pending_reminder|pending_action|merged states = Ticket::State.by_category(:open) # :open|:closed|:work_on|:work_on_all|:viewable|:pending_reminder|:pending_action|:merged
returns: returns:
state objects state object list
=end =end
def self.by_category(category) def self.by_category(category)
if category == 'open'
return Ticket::State.where( case category.to_sym
state_type_id: Ticket::StateType.where(name: ['new', 'open', 'pending reminder', 'pending action']) when :open
) state_types = ['new', 'open', 'pending reminder', 'pending action']
elsif category == 'pending_reminder' when :pending_reminder
return Ticket::State.where( state_types = ['pending reminder']
state_type_id: Ticket::StateType.where(name: ['pending reminder']) when :pending_action
) state_types = ['pending action']
elsif category == 'pending_action' when :work_on
return Ticket::State.where( state_types = %w(new open)
state_type_id: Ticket::StateType.where(name: ['pending action']) when :work_on_all
) state_types = ['new', 'open', 'pending reminder']
elsif category == 'work_on' when :viewable
return Ticket::State.where( state_types = ['new', 'open', 'pending reminder', 'pending action', 'closed', 'removed']
state_type_id: Ticket::StateType.where(name: %w(new open)) when :closed
) state_types = %w(closed)
elsif category == 'work_on_all' when :merged
return Ticket::State.where( state_types = %w(merged)
state_type_id: Ticket::StateType.where(name: ['new', 'open', 'pending reminder'])
)
elsif category == 'viewable'
return Ticket::State.where(
state_type_id: Ticket::StateType.where(name: ['new', 'open', 'pending reminder', 'pending action', 'closed', 'removed'])
)
elsif category == 'closed'
return Ticket::State.where(
state_type_id: Ticket::StateType.where(name: %w(closed))
)
elsif category == 'merged'
return Ticket::State.where(
state_type_id: Ticket::StateType.where(name: %w(merged))
)
end end
raise "Unknown category '#{category}'"
raise "Unknown category '#{category}'" if state_types.blank?
Ticket::State.where(
state_type_id: Ticket::StateType.where(name: state_types)
)
end end
=begin =begin

View file

@ -3311,7 +3311,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('open').pluck(:id), value: Ticket::State.by_category(:open).pluck(:id),
}, },
'ticket.owner_id' => { 'ticket.owner_id' => {
operator: 'is', operator: 'is',
@ -3338,7 +3338,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('work_on_all').pluck(:id), value: Ticket::State.by_category(:work_on_all).pluck(:id),
}, },
'ticket.owner_id' => { 'ticket.owner_id' => {
operator: 'is', operator: 'is',
@ -3365,7 +3365,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('pending_reminder').pluck(:id), value: Ticket::State.by_category(:pending_reminder).pluck(:id),
}, },
'ticket.owner_id' => { 'ticket.owner_id' => {
operator: 'is', operator: 'is',
@ -3397,7 +3397,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('work_on_all').pluck(:id), value: Ticket::State.by_category(:work_on_all).pluck(:id),
}, },
}, },
order: { order: {
@ -3420,7 +3420,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('pending_reminder').pluck(:id), value: Ticket::State.by_category(:pending_reminder).pluck(:id),
}, },
'ticket.pending_time' => { 'ticket.pending_time' => {
operator: 'within next (relative)', operator: 'within next (relative)',
@ -3473,7 +3473,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('viewable').pluck(:id), value: Ticket::State.by_category(:viewable).pluck(:id),
}, },
'ticket.customer_id' => { 'ticket.customer_id' => {
operator: 'is', operator: 'is',
@ -3500,7 +3500,7 @@ Overview.create_if_not_exists(
condition: { condition: {
'ticket.state_id' => { 'ticket.state_id' => {
operator: 'is', operator: 'is',
value: Ticket::State.by_category('viewable').pluck(:id), value: Ticket::State.by_category(:viewable).pluck(:id),
}, },
'ticket.organization_id' => { 'ticket.organization_id' => {
operator: 'is', operator: 'is',

View file

@ -4,7 +4,7 @@ class Stats::TicketEscalation
def self.generate(user) def self.generate(user)
open_state_ids = Ticket::State.by_category('open').map(&:id) open_state_ids = Ticket::State.by_category(:open).pluck(:id)
# get users groups # get users groups
group_ids = user.groups.map(&:id) group_ids = user.groups.map(&:id)

View file

@ -5,26 +5,26 @@ class Stats::TicketInProcess
def self.generate(user) def self.generate(user)
# get own tickets which are "workable" # get own tickets which are "workable"
open_state_ids = Ticket::State.by_category('work_on').map(&:id) open_state_ids = Ticket::State.by_category(:work_on).pluck(:id)
pending_state_ids = Ticket::State.by_category('pending_reminder').map(&:id) pending_state_ids = Ticket::State.by_category(:pending_reminder).pluck(:id)
own_ticket_ids = Ticket.select('id').where( own_ticket_ids = Ticket.select('id').where(
'owner_id = ? AND (state_id IN (?) OR (state_id IN (?) AND pending_time < ?))', 'owner_id = ? AND (state_id IN (?) OR (state_id IN (?) AND pending_time < ?))',
user.id, open_state_ids, pending_state_ids, Time.zone.now user.id, open_state_ids, pending_state_ids, Time.zone.now
).limit(1000).map(&:id) ).limit(1000).pluck(:id)
# get all tickets where I worked on today (owner & closed today) # get all tickets where I worked on today (owner & closed today)
closed_state_ids = Ticket::State.by_category('closed').map(&:id) closed_state_ids = Ticket::State.by_category(:closed).pluck(:id)
closed_ticket_ids = Ticket.select('id').where( closed_ticket_ids = Ticket.select('id').where(
'owner_id = ? AND state_id IN (?) AND close_at > ?', 'owner_id = ? AND state_id IN (?) AND close_at > ?',
user.id, closed_state_ids, Time.zone.now - 1.day user.id, closed_state_ids, Time.zone.now - 1.day
).limit(100).map(&:id) ).limit(100).pluck(:id)
# get all tickets which I changed to pending action # get all tickets which I changed to pending action
pending_action_state_ids = Ticket::State.by_category('pending_action').map(&:id) pending_action_state_ids = Ticket::State.by_category(:pending_action).pluck(:id)
pending_action_ticket_ids = Ticket.select('id').where( pending_action_ticket_ids = Ticket.select('id').where(
'owner_id = ? AND state_id IN (?) AND updated_at > ?', 'owner_id = ? AND state_id IN (?) AND updated_at > ?',
user.id, pending_action_state_ids, Time.zone.now - 1.day user.id, pending_action_state_ids, Time.zone.now - 1.day
).limit(100).map(&:id) ).limit(100).pluck(:id)
all_ticket_ids = own_ticket_ids.concat(closed_ticket_ids).concat(pending_action_ticket_ids).uniq all_ticket_ids = own_ticket_ids.concat(closed_ticket_ids).concat(pending_action_ticket_ids).uniq

View file

@ -4,7 +4,7 @@ class Stats::TicketLoadMeasure
def self.generate(user) def self.generate(user)
open_state_ids = Ticket::State.by_category('open').map(&:id) open_state_ids = Ticket::State.by_category(:open).pluck(:id)
# owned tickets # owned tickets
count = Ticket.where(owner_id: user.id, state_id: open_state_ids).count count = Ticket.where(owner_id: user.id, state_id: open_state_ids).count

View file

@ -0,0 +1,18 @@
require 'rails_helper'
RSpec.describe Ticket::State do
context '.by_category' do
it 'looks up states by category' do
result = described_class.by_category(:open)
expect(result).to be_an(ActiveRecord::Relation)
expect(result).to_not be_empty
expect(result.first).to be_a(Ticket::State)
end
it 'raises RuntimeError for invalid category' do
expect { described_class.by_category(:invalidcategoryname) }.to raise_error(RuntimeError)
end
end
end