From cc326ad93b1cd6db217954b392c1ddb29c615939 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Wed, 15 Aug 2018 17:16:41 +0200 Subject: [PATCH] Follow up for issue #2171 - Overview not showing unassigned tickets "if not defined" (e. g. for owner but also for select or input fields). --- app/models/ticket.rb | 27 +- test/integration/object_manager_test.rb | 327 ++++++++++++++++++++++++ test/unit/ticket_selector_test.rb | 1 - 3 files changed, 350 insertions(+), 5 deletions(-) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index f7df061df..3fb95b8ae 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -465,7 +465,7 @@ condition example value: '2015-10-17T06:00:00.000Z', }, 'ticket.created_at' => { - operator: 'within next (relative)', # before,within,in,after + operator: 'within next (relative)', # within next, within last, after, before range: 'day', # minute|hour|day|month|year value: '25', }, @@ -544,9 +544,12 @@ condition example raise "Invalid selector #{selector_raw.inspect}" if !selector_raw.respond_to?(:key?) selector = selector_raw.stringify_keys raise "Invalid selector, operator missing #{selector.inspect}" if !selector['operator'] + raise "Invalid selector, operator #{selector['operator']} is invalid #{selector.inspect}" if selector['operator'] !~ /^(is|is\snot|contains|contains\s(not|all|one|all\snot|one\snot)|(after|before)\s\(absolute\)|(within\snext|within\slast|after|before)\s\(relative\))$/ # validate value / allow blank but only if pre_condition exists and is not specific - if !selector.key?('value') || ((selector['value'].class == String || selector['value'].class == Array) && (selector['value'].respond_to?(:blank?) && selector['value'].blank?)) + if !selector.key?('value') || + (selector['value'].class == Array && selector['value'].respond_to?(:blank?) && selector['value'].blank?) || + (selector['operator'] =~ /^contains/ && selector['value'].respond_to?(:blank?) && selector['value'].blank?) return nil if selector['pre_condition'].nil? return nil if selector['pre_condition'].respond_to?(:blank?) && selector['pre_condition'].blank? return nil if selector['pre_condition'] == 'specific' @@ -598,10 +601,18 @@ condition example if selector['value'].nil? query += "#{attribute} IS NULL" else - query += "#{attribute} IN (?)" if attributes[1] == 'out_of_office_replacement_id' + query += "#{attribute} IN (?)" bind_params.push User.find(selector['value']).out_of_office_agent_of.pluck(:id) else + if selector['value'].class != Array + selector['value'] = [selector['value']] + end + query += if selector['value'].include?('') + "(#{attribute} IN (?) OR #{attribute} IS NULL)" + else + "#{attribute} IN (?)" + end bind_params.push selector['value'] end end @@ -631,10 +642,18 @@ condition example if selector['value'].nil? query += "#{attribute} IS NOT NULL" else - query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))" if attributes[1] == 'out_of_office_replacement_id' bind_params.push User.find(selector['value']).out_of_office_agent_of.pluck(:id) + query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))" else + if selector['value'].class != Array + selector['value'] = [selector['value']] + end + query += if selector['value'].include?('') + "(#{attribute} IS NOT NULL AND #{attribute} NOT IN (?))" + else + "(#{attribute} IS NULL OR #{attribute} NOT IN (?))" + end bind_params.push selector['value'] end end diff --git a/test/integration/object_manager_test.rb b/test/integration/object_manager_test.rb index 972345819..8bde45c19 100644 --- a/test/integration/object_manager_test.rb +++ b/test/integration/object_manager_test.rb @@ -913,4 +913,331 @@ class ObjectManagerTest < ActiveSupport::TestCase end + test 'overview any owner / no owner is set' do + + group = Group.create!( + name: 'OverviewTest', + updated_at: '2015-02-05 16:37:00', + updated_by_id: 1, + created_by_id: 1, + ) + roles = Role.where(name: 'Agent') + agent1 = User.create!( + login: 'ticket-overview-agent1@example.com', + firstname: 'Overview', + lastname: 'Agent1', + email: 'ticket-overview-agent1@example.com', + password: 'agentpw', + active: true, + roles: roles, + groups: [group], + updated_at: '2015-02-05 16:37:00', + updated_by_id: 1, + created_by_id: 1, + ) + + attribute1 = ObjectManager::Attribute.add( + object: 'Ticket', + name: 'watcher', + display: 'watcher', + data_type: 'select', + data_option: { + default: '', + maxlength: 200, + type: 'text', + null: true, + options: { + aa: 'agent a', + bb: 'agent b', + cc: 'agent c', + }, + }, + active: true, + screens: {}, + position: 20, + created_by_id: 1, + updated_by_id: 1, + ) + + assert_equal(true, ObjectManager::Attribute.pending_migration?) + assert_equal(1, ObjectManager::Attribute.migrations.count) + + assert(ObjectManager::Attribute.migration_execute) + + Ticket.destroy_all + Overview.destroy_all + + UserInfo.current_user_id = 1 + overview_role = Role.find_by(name: 'Agent') + overview1 = Overview.create!( + name: 'not watched', + prio: 1000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is', + value: '', + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview2 = Overview.create!( + name: 'not watched by somebody', + prio: 2000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is not', + value: '', + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview3 = Overview.create!( + name: 'not watched as array', + prio: 3000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is', + value: [''], + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview4 = Overview.create!( + name: 'not watched by somebody as array', + prio: 4000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is not', + value: [''], + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview5 = Overview.create!( + name: 'watched by aa', + prio: 5000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is', + value: 'aa', + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview6 = Overview.create!( + name: 'not watched by aa', + prio: 6000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is not', + value: 'aa', + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview7 = Overview.create!( + name: 'watched by aa array', + prio: 7000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is', + value: ['aa'], + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + overview8 = Overview.create!( + name: 'not watched by aa array', + prio: 8000, + role_ids: [overview_role.id], + condition: { + 'ticket.watcher' => { + operator: 'is not', + value: ['aa'], + }, + }, + order: { + by: 'created_at', + direction: 'ASC', + }, + view: { + d: %w[title customer group created_at], + s: %w[title customer group created_at], + m: %w[number title customer group created_at], + view_mode_default: 's', + }, + ) + + ticket1 = Ticket.create!( + title: 'overview test 1', + group: Group.lookup(name: 'OverviewTest'), + customer_id: 2, + owner_id: 1, + watcher: '', + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + ) + + travel 2.seconds + ticket2 = Ticket.create!( + title: 'overview test 2', + group: Group.lookup(name: 'OverviewTest'), + customer_id: 2, + owner_id: nil, + watcher: nil, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + ) + + travel 2.seconds + ticket3 = Ticket.create!( + title: 'overview test 3', + group: Group.lookup(name: 'OverviewTest'), + customer_id: 2, + owner_id: agent1.id, + watcher: 'aa', + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + ) + + result = Ticket::Overviews.index(agent1) + assert_equal(result[0][:overview][:id], overview1.id) + assert_equal(result[0][:overview][:name], 'not watched') + assert_equal(result[0][:overview][:view], 'not_watched') + assert_equal(result[0][:tickets].class, Array) + assert_equal(result[0][:tickets][0][:id], ticket1.id) + assert_equal(result[0][:tickets][1][:id], ticket2.id) + assert_equal(result[0][:count], 2) + + assert_equal(result[1][:overview][:id], overview2.id) + assert_equal(result[1][:overview][:name], 'not watched by somebody') + assert_equal(result[1][:overview][:view], 'not_watched_by_somebody') + assert_equal(result[1][:tickets].class, Array) + assert_equal(result[1][:tickets][0][:id], ticket3.id) + assert_equal(result[1][:count], 1) + + assert_equal(result[2][:overview][:id], overview3.id) + assert_equal(result[2][:overview][:name], 'not watched as array') + assert_equal(result[2][:overview][:view], 'not_watched_as_array') + assert_equal(result[2][:tickets].class, Array) + assert_equal(result[2][:tickets][0][:id], ticket1.id) + assert_equal(result[2][:tickets][1][:id], ticket2.id) + assert_equal(result[2][:count], 2) + + assert_equal(result[3][:overview][:id], overview4.id) + assert_equal(result[3][:overview][:name], 'not watched by somebody as array') + assert_equal(result[3][:overview][:view], 'not_watched_by_somebody_as_array') + assert_equal(result[3][:tickets].class, Array) + assert_equal(result[3][:tickets][0][:id], ticket3.id) + assert_equal(result[3][:count], 1) + + assert_equal(result[4][:overview][:id], overview5.id) + assert_equal(result[4][:overview][:name], 'watched by aa') + assert_equal(result[4][:overview][:view], 'watched_by_aa') + assert_equal(result[4][:tickets].class, Array) + assert_equal(result[4][:tickets][0][:id], ticket3.id) + assert_equal(result[4][:count], 1) + + assert_equal(result[5][:overview][:id], overview6.id) + assert_equal(result[5][:overview][:name], 'not watched by aa') + assert_equal(result[5][:overview][:view], 'not_watched_by_aa') + assert_equal(result[5][:tickets].class, Array) + assert_equal(result[5][:tickets][0][:id], ticket1.id) + assert_equal(result[5][:tickets][1][:id], ticket2.id) + assert_equal(result[5][:count], 2) + + assert_equal(result[6][:overview][:id], overview7.id) + assert_equal(result[6][:overview][:name], 'watched by aa array') + assert_equal(result[6][:overview][:view], 'watched_by_aa_array') + assert_equal(result[6][:tickets].class, Array) + assert_equal(result[6][:tickets][0][:id], ticket3.id) + assert_equal(result[6][:count], 1) + + assert_equal(result[7][:overview][:id], overview8.id) + assert_equal(result[7][:overview][:name], 'not watched by aa array') + assert_equal(result[7][:overview][:view], 'not_watched_by_aa_array') + assert_equal(result[7][:tickets].class, Array) + assert_equal(result[7][:tickets][0][:id], ticket1.id) + assert_equal(result[7][:tickets][1][:id], ticket2.id) + assert_equal(result[7][:count], 2) + + end + end diff --git a/test/unit/ticket_selector_test.rb b/test/unit/ticket_selector_test.rb index 6ba4e33e7..b0a6787e0 100644 --- a/test/unit/ticket_selector_test.rb +++ b/test/unit/ticket_selector_test.rb @@ -207,7 +207,6 @@ class TicketSelectorTest < ActiveSupport::TestCase }, 'ticket.state_id' => { operator: 'is', - value: '', }, }