Fixes #2694 - Zendesk import doesn't consider all tickets.

This commit is contained in:
Martin Edenhofer 2021-01-21 08:22:17 +01:00 committed by Thorsten Eckel
parent b8ea658495
commit 708b953d81
4 changed files with 84 additions and 58 deletions

View file

@ -9,10 +9,11 @@ class Sequencer
def self.included(base) def self.included(base)
base.uses :client base.uses :client
base.provides :import_job
end end
def resource_collection def resource_collection
client.send resource_klass.pluralize.underscore @resource_collection ||= "::ZendeskAPI::#{resource_klass}".constantize.incremental_export(client, 1)
end end
def resource_iteration def resource_iteration
@ -33,7 +34,8 @@ class Sequencer
# per request # per request
def update_count def update_count
update_import_job update_import_job
self.previous_page = next_page
self.previous_page = current_page
end end
def update_import_job def update_import_job
@ -43,36 +45,33 @@ class Sequencer
end end
def klass_key def klass_key
resource_klass.singularize.to_sym @klass_key ||= resource_klass.pluralize.to_sym
end end
def updated_import_job def updated_import_job
import_job.result[klass_key][:total] += current_request_count
import_job.result[klass_key].merge(
total: import_job.result[klass_key][:total] + current_request_count
)
import_job import_job
end end
def update_required? def update_required?
return false if previous_page.blank? # means: still on first page
return false if previous_page == next_page return false if current_page.blank?
current_request_count.present? previous_page != current_page
end end
def current_request_count def current_request_count
# access the internal instance method of the # access the internal instance method of the
# Zendesk collection request to get the current # Zendesk collection request to get the current
# count of the endpoint (max. 1000) # count of the fetched result (max. 1000)
resource_collection_attribute.instance_variable_get(:@count) resource_collection.fetch.size
end end
def next_page def current_page
# access the internal instance method of the # access the internal instance method of the
# Zendesk collection request to get the next # Zendesk collection request to get the current
# page number of the endpoint # page number of the endpoint
resource_collection_attribute.instance_variable_get(:@next_page) resource_collection.instance_variable_get(:@query)
end end
end end
end end

View file

@ -11,19 +11,30 @@ class Sequencer
def statistics_diff def statistics_diff
%i[Groups Users Organizations Tickets].each_with_object({}) do |object, stats| %i[Groups Users Organizations Tickets].each_with_object({}) do |object, stats|
stats[object] = object_diff(object) stats[object] = empty_diff.merge(
end total: request(object).count!
end
def object_diff(object)
collection_name = object.to_s.underscore
collection = client.send collection_name
empty_diff.merge(
total: collection.count!
) )
end end
end end
# the special "incremental_export" logic is needed because Zendesk
# archives records and doesn't return them via e.g. client.tickets
# endpoint as described here:
# https://github.com/zammad/zammad/issues/558#issuecomment-267951351
# Counting via the incremental_export endpoint has the limitations
# that it returns max. 1000. That's why we need to update the total
# number while importing in the resource loop
def request(object)
resource_class = "::ZendeskAPI::#{object.to_s.singularize}".safe_constantize
if resource_class.respond_to?(:incremental_export)
# read as: ::ZendeskAPI::Ticket.incremental_export(client, 1)
resource_class.incremental_export(client, 1)
else
# read as: client.groups
client.send(object.to_s.underscore.to_sym)
end
end
end
end end
end end
end end

View file

@ -61,7 +61,7 @@ class Sequencer
end end
def resource_collection def resource_collection
collection_provider.public_send(resource_collection_attribute) @resource_collection ||= collection_provider.public_send(resource_collection_attribute)
end end
def resource_iteration_method def resource_iteration_method

View file

@ -38,26 +38,6 @@ class ZendeskImportTest < ActiveSupport::TestCase
total: 2 total: 2
}, },
Users: { Users: {
skipped: 0,
created: 141,
updated: 0,
unchanged: 0,
failed: 0,
deactivated: 0,
sum: 141,
total: 141
},
Organizations: {
skipped: 0,
created: 1,
updated: 0,
unchanged: 0,
failed: 0,
deactivated: 0,
sum: 1,
total: 1
},
Tickets: {
skipped: 0, skipped: 0,
created: 141, created: 141,
updated: 1, updated: 1,
@ -66,6 +46,26 @@ class ZendeskImportTest < ActiveSupport::TestCase
deactivated: 0, deactivated: 0,
sum: 142, sum: 142,
total: 142 total: 142
},
Organizations: {
skipped: 0,
created: 1,
updated: 0,
unchanged: 1,
failed: 0,
deactivated: 0,
sum: 2,
total: 2
},
Tickets: {
skipped: 1,
created: 142,
updated: 2,
unchanged: 0,
failed: 0,
deactivated: 0,
sum: 145,
total: 145
} }
} }
@ -78,8 +78,8 @@ class ZendeskImportTest < ActiveSupport::TestCase
assert_equal(3, Group.count, 'groups') assert_equal(3, Group.count, 'groups')
assert_equal(3, Role.count, 'roles') assert_equal(3, Role.count, 'roles')
assert_equal(2, Organization.count, 'organizations') assert_equal(2, Organization.count, 'organizations')
assert_equal(142, Ticket.count, 'tickets') assert_equal(143, Ticket.count, 'tickets')
assert_equal(151, Ticket::Article.count, 'ticket articles') assert_equal(153, Ticket::Article.count, 'ticket articles')
assert_equal(3, Store.count, 'ticket article attachments') assert_equal(3, Store.count, 'ticket article attachments')
# TODO: Macros, Views, Automations... # TODO: Macros, Views, Automations...
@ -97,9 +97,9 @@ class ZendeskImportTest < ActiveSupport::TestCase
checks = [ checks = [
{ {
id: 5, id: 144,
data: { data: {
firstname: 'Bob', firstname: 'Bob Smith',
lastname: 'Smith', lastname: 'Smith',
login: 'bob.smith@znuny.com', login: 'bob.smith@znuny.com',
email: 'bob.smith@znuny.com', email: 'bob.smith@znuny.com',
@ -111,7 +111,7 @@ class ZendeskImportTest < ActiveSupport::TestCase
groups: [group_support], groups: [group_support],
}, },
{ {
id: 6, id: 142,
data: { data: {
firstname: 'Hansimerkur', firstname: 'Hansimerkur',
lastname: '', lastname: '',
@ -124,7 +124,7 @@ class ZendeskImportTest < ActiveSupport::TestCase
groups: [group_additional_group, group_support], groups: [group_additional_group, group_support],
}, },
{ {
id: 7, id: 6,
data: { data: {
firstname: 'Bernd', firstname: 'Bernd',
lastname: 'Hofbecker', lastname: 'Hofbecker',
@ -136,7 +136,7 @@ class ZendeskImportTest < ActiveSupport::TestCase
groups: [], groups: [],
}, },
{ {
id: 8, id: 143,
data: { data: {
firstname: 'Zendesk', firstname: 'Zendesk',
lastname: '', lastname: '',
@ -148,7 +148,7 @@ class ZendeskImportTest < ActiveSupport::TestCase
groups: [], groups: [],
}, },
{ {
id: 90, id: 5,
data: { data: {
firstname: 'Hans', firstname: 'Hans',
lastname: 'Peter Wurst', lastname: 'Peter Wurst',
@ -336,7 +336,7 @@ class ZendeskImportTest < ActiveSupport::TestCase
group_id: 3, group_id: 3,
priority_id: 3, priority_id: 3,
owner_id: User.find_by(login: 'bob.smith@znuny.com').id, owner_id: User.find_by(login: 'bob.smith@znuny.com').id,
customer_id: 7, customer_id: User.find_by(login: 'bernd.hofbecker@znuny.com').id,
organization_id: 2, organization_id: 2,
test_checkbox: true, test_checkbox: true,
custom_integer: 999, custom_integer: 999,
@ -357,7 +357,7 @@ class ZendeskImportTest < ActiveSupport::TestCase
group_id: 3, group_id: 3,
priority_id: 1, priority_id: 1,
owner_id: User.find_by(login: 'bob.smith@znuny.com').id, owner_id: User.find_by(login: 'bob.smith@znuny.com').id,
customer_id: 8, customer_id: User.find_by(login: 'noreply@zendesk.com').id,
organization_id: nil, organization_id: nil,
test_checkbox: false, test_checkbox: false,
custom_integer: nil, custom_integer: nil,
@ -377,8 +377,8 @@ class ZendeskImportTest < ActiveSupport::TestCase
state_id: 1, state_id: 1,
group_id: 3, group_id: 3,
priority_id: 2, priority_id: 2,
owner_id: 1, owner_id: User.find_by(login: '-').id,
customer_id: 92, customer_id: 69,
organization_id: nil, organization_id: nil,
}, },
}, },
@ -393,11 +393,27 @@ class ZendeskImportTest < ActiveSupport::TestCase
state_id: 1, state_id: 1,
group_id: 1, group_id: 1,
priority_id: 2, priority_id: 2,
owner_id: 1, owner_id: User.find_by(login: '-').id,
customer_id: 144, customer_id: 7,
organization_id: nil, organization_id: nil,
}, },
}, },
{
id: 145,
data: {
title: 'closed ticket - should be archived and imported',
note: nil,
create_article_type_id: 11,
create_article_sender_id: 1,
article_count: 2,
state_id: Ticket::State.find_by(name: 'closed').id,
group_id: Group.find_by(name: 'Additional Group').id,
priority_id: Ticket::Priority.find_by(name: '2 normal').id,
owner_id: User.find_by(login: 'hansimerkur@znuny.com').id,
customer_id: User.find_by(login: 'bob.smith@znuny.com').id,
organization_id: Organization.find_by(name: 'Znuny').id,
},
},
# { # {
# id: , # id: ,
# data: { # data: {