From 4bd9dcf5181cad36d2b1a8a543ce07c9bc7d5274 Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Fri, 9 Feb 2018 16:46:55 +0100 Subject: [PATCH] Fixed issue #1769 - Ticket shown multiple times in overview. --- app/models/ticket.rb | 8 +++--- app/models/ticket/overviews.rb | 16 ++++++------ spec/factories/overview.rb | 34 +++++++++++++++++++++++++ spec/models/ticket/overviews_spec.rb | 37 ++++++++++++++++++++++++++++ spec/models/ticket_spec.rb | 29 ++++++++++++++++++++++ 5 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 spec/factories/overview.rb create mode 100644 spec/models/ticket/overviews_spec.rb diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 4883e8764..3fdfe0c92 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -424,14 +424,14 @@ get count of tickets and tickets which match on selector ActiveRecord::Base.transaction(requires_new: true) do begin if !current_user - ticket_count = Ticket.where(query, *bind_params).joins(tables).count - tickets = Ticket.where(query, *bind_params).joins(tables).limit(limit) + ticket_count = Ticket.distinct.where(query, *bind_params).joins(tables).count + tickets = Ticket.distinct.where(query, *bind_params).joins(tables).limit(limit) return [ticket_count, tickets] end access_condition = Ticket.access_condition(current_user, access) - ticket_count = Ticket.where(access_condition).where(query, *bind_params).joins(tables).count - tickets = Ticket.where(access_condition).where(query, *bind_params).joins(tables).limit(limit) + ticket_count = Ticket.distinct.where(access_condition).where(query, *bind_params).joins(tables).count + tickets = Ticket.distinct.where(access_condition).where(query, *bind_params).joins(tables).limit(limit) return [ticket_count, tickets] rescue ActiveRecord::StatementInvalid => e diff --git a/app/models/ticket/overviews.rb b/app/models/ticket/overviews.rb index 393490f19..e88a1740d 100644 --- a/app/models/ticket/overviews.rb +++ b/app/models/ticket/overviews.rb @@ -113,23 +113,21 @@ returns end end - ticket_result = Ticket.select('id, updated_at') + ticket_result = Ticket.distinct .where(access_condition) .where(query_condition, *bind_condition) .joins(tables) .order(order_by) .limit(1000) - .pluck(:id, :updated_at) - tickets = [] - ticket_result.each do |ticket| - ticket_item = { - id: ticket[0], - updated_at: ticket[1], + tickets = ticket_result.map do |ticket| + { + id: ticket[:id], + updated_at: ticket[:updated_at], } - tickets.push ticket_item end - count = Ticket.where(access_condition).where(query_condition, *bind_condition).joins(tables).count() + + count = Ticket.distinct.where(access_condition).where(query_condition, *bind_condition).joins(tables).count() item = { overview: { name: overview.name, diff --git a/spec/factories/overview.rb b/spec/factories/overview.rb new file mode 100644 index 000000000..71de26af2 --- /dev/null +++ b/spec/factories/overview.rb @@ -0,0 +1,34 @@ +FactoryBot.define do + + factory :overview do + name 'My Factory Tickets' + link 'my_factory_tickets' + prio 1100 + role_ids { [ Role.find_by(name: 'Customer').id, Role.find_by(name: 'Agent').id, Role.find_by(name: 'Admin').id ] } + out_of_office true + condition do + { + 'ticket.state_id' => { + operator: 'is', + value: [ Ticket::State.lookup(name: 'new').id, Ticket::State.lookup(name: 'open').id ], + }, + } + end + order do + { + by: 'created_at', + direction: 'DESC', + } + end + view do + { + d: %w[title customer state created_at], + s: %w[number title state created_at], + m: %w[number title state created_at], + view_mode_default: 's', + } + end + updated_by_id 1 + created_by_id 1 + end +end diff --git a/spec/models/ticket/overviews_spec.rb b/spec/models/ticket/overviews_spec.rb new file mode 100644 index 000000000..2ba80203d --- /dev/null +++ b/spec/models/ticket/overviews_spec.rb @@ -0,0 +1,37 @@ +require 'rails_helper' + +RSpec.describe Ticket::Overviews do + + describe '#index' do + + # https://github.com/zammad/zammad/issues/1769 + it 'does not return multiple results for a single ticket' do + user = create(:user) + source_ticket = create(:ticket, customer: user, created_by_id: user.id) + source_ticket2 = create(:ticket, customer: user, created_by_id: user.id) + + # create some articles + create(:ticket_article, ticket_id: source_ticket.id, from: 'asdf1@blubselector.de', created_by_id: user.id) + create(:ticket_article, ticket_id: source_ticket.id, from: 'asdf2@blubselector.de', created_by_id: user.id) + create(:ticket_article, ticket_id: source_ticket.id, from: 'asdf3@blubselector.de', created_by_id: user.id) + create(:ticket_article, ticket_id: source_ticket2.id, from: 'asdf3@blubselector.de', created_by_id: user.id) + create(:ticket_article, ticket_id: source_ticket2.id, from: 'asdf4@blubselector.de', created_by_id: user.id) + create(:ticket_article, ticket_id: source_ticket2.id, from: 'asdf5@blubselector.de', created_by_id: user.id) + + condition = { + 'article.from' => { + operator: 'contains', + value: 'blubselector.de', + }, + } + overview = create(:overview, condition: condition) + + result = Ticket::Overviews.index(user) + result = result.select { |x| x[:overview][:name] == 'My Factory Tickets' } + + expect(result.count).to be == 1 + expect(result[0][:count]).to be == 2 + expect(result[0][:tickets].count).to be == 2 + end + end +end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 0b8767ba6..dbbb6e28d 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -203,6 +203,35 @@ RSpec.describe Ticket do end + describe '#selectors' do + + # https://github.com/zammad/zammad/issues/1769 + it 'does not return multiple results for a single ticket' do + source_ticket = create(:ticket) + source_ticket2 = create(:ticket) + + # create some articles + create(:ticket_article, ticket_id: source_ticket.id, from: 'asdf1@blubselector.de') + create(:ticket_article, ticket_id: source_ticket.id, from: 'asdf2@blubselector.de') + create(:ticket_article, ticket_id: source_ticket.id, from: 'asdf3@blubselector.de') + create(:ticket_article, ticket_id: source_ticket2.id, from: 'asdf4@blubselector.de') + create(:ticket_article, ticket_id: source_ticket2.id, from: 'asdf5@blubselector.de') + create(:ticket_article, ticket_id: source_ticket2.id, from: 'asdf6@blubselector.de') + + condition = { + 'article.from' => { + operator: 'contains', + value: 'blubselector.de', + }, + } + + ticket_count, tickets = Ticket.selectors(condition, 100, nil, 'full') + + expect(ticket_count).to be == 2 + expect(tickets.count).to be == 2 + end + end + context 'callbacks' do describe '#reset_pending_time' do