From 901cc3fadf47bfe16a92039a747801c6aebfb66b Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Wed, 4 Jul 2018 18:07:25 +0800 Subject: [PATCH] Improve NULL handling to Ticket#selector2sql (fixes #1316) --- app/models/ticket.rb | 8 ++--- test/unit/ticket_trigger_test.rb | 51 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 66a7fad0d..54bcb9506 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -610,20 +610,20 @@ condition example elsif selector['operator'] == 'is not' if selector['pre_condition'] == 'not_set' if attributes[1].match?(/^(created_by|updated_by|owner|customer|user)_id/) - query += "#{attribute} NOT IN (?)" + query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))" bind_params.push 1 else query += "#{attribute} IS NOT NULL" end elsif selector['pre_condition'] == 'current_user.id' - query += "#{attribute} NOT IN (?)" + query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))" if attributes[1] == 'out_of_office_replacement_id' bind_params.push User.find(current_user_id).out_of_office_agent_of.pluck(:id) else bind_params.push current_user_id end elsif selector['pre_condition'] == 'current_user.organization_id' - query += "#{attribute} NOT IN (?)" + query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))" user = User.find_by(id: current_user_id) bind_params.push user.organization_id else @@ -631,7 +631,7 @@ condition example if selector['value'].nil? query += "#{attribute} IS NOT NULL" else - query += "#{attribute} NOT IN (?)" + 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) else diff --git a/test/unit/ticket_trigger_test.rb b/test/unit/ticket_trigger_test.rb index af5487cb7..088d38ac9 100644 --- a/test/unit/ticket_trigger_test.rb +++ b/test/unit/ticket_trigger_test.rb @@ -4315,4 +4315,55 @@ class TicketTriggerTest < ActiveSupport::TestCase assert_equal('789', article1.attachments[0].size) assert_equal('text/html', article1.content_type) end + + # Issue #1316 - 'organization is not X' conditions break triggers + test 'NOT IN predicates handle NULL values' do + customer = User.create!( + email: 'issue_1316_test_user@zammad.org', + created_by_id: 1, + updated_by_id: 1, + ) + + Trigger.create_or_update( + name: 'auto reply (condition: organization-is-not)', + condition: { + 'ticket.organization_id' => { + 'operator' => 'is not', + 'value' => Organization.first.id.to_s, + }, + }, + perform: { + 'notification.email' => { + 'body' => 'Lorem ipsum dolor', + 'recipient' => 'ticket_customer', + 'subject' => 'Thanks for your inquiry (#{ticket.title})!', + }, + }, + active: true, + created_by_id: 1, + updated_by_id: 1, + ) + + ticket = Ticket.create!( + title: "some title\n äöüß", + group: Group.lookup(name: 'Users'), + customer: customer, + updated_by_id: 1, + created_by_id: 1, + ) + + assert_nil(customer.organization_id) + assert_equal(0, ticket.reload.articles.count, 'ticket.articles verify') + + Observer::Transaction.commit + + assert_equal(1, ticket.reload.articles.count, 'ticket.articles verify') + + autoreply = ticket.articles.first + assert_equal('Zammad ', autoreply.from) + assert_equal(customer.email, autoreply.to) + assert_equal("Thanks for your inquiry (#{ticket.title})!", autoreply.subject) + assert_equal('Lorem ipsum dolor', autoreply.body) + assert_equal('text/html', autoreply.content_type) + end end