Sutty/trabajo-afectivo
5
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fixes #3681 - Freshdesk import works not with more then 30_000 tickets.

Browse source
This commit is contained in:
Thorsten Eckel 2021-08-12 14:04:45 +02:00
parent 29f98be91f
commit c89ed949de
22 changed files with 1848 additions and 1540 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
lib/sequencer/sequence/import/freshdesk/agent.rb
View file

@ -11,7 +11,7 @@ class Sequencer
'Common::ModelClass::User',
'Import::Freshdesk::Agent::Mapping',
'Import::Common::Model::Attributes::AddByIds',
'Import::Common::Model::FindBy::Name',
'Import::Common::Model::FindBy::UserAttributes',
'Import::Common::Model::Update',
'Import::Common::Model::Create',
'Import::Common::Model::Save',

2
lib/sequencer/sequence/import/freshdesk/contact.rb
View file

@ -12,7 +12,7 @@ class Sequencer
'Import::Freshdesk::Contact::Mapping',
'Import::Freshdesk::Mapping::CustomFields',
'Import::Common::Model::Attributes::AddByIds',
'Import::Common::Model::FindBy::Name',
'Import::Common::Model::FindBy::UserAttributes',
'Import::Common::Model::Update',
'Import::Common::Model::Create',
'Import::Common::Model::Save',

1
lib/sequencer/sequence/import/freshdesk/conversation.rb
View file

@ -11,6 +11,7 @@ class Sequencer
'Common::ModelClass::Ticket::Article',
'Import::Freshdesk::Conversation::Mapping',
'Import::Freshdesk::Conversation::InlineImages',
'Import::Common::Model::FindBy::MessageId',
'Import::Common::Model::Update',
'Import::Common::Model::Create',
'Import::Common::Model::Save',

1
lib/sequencer/sequence/import/freshdesk/description.rb
View file

@ -13,6 +13,7 @@ class Sequencer
# Handling of inline images and attachments is the same for first article (description)
# and subsequent articles (conversation).
'Import::Freshdesk::Conversation::InlineImages',
'Import::Common::Model::FindBy::MessageId',
'Import::Common::Model::Update',
'Import::Common::Model::Create',
'Import::Common::Model::Save',

1
lib/sequencer/sequence/import/freshdesk/ticket.rb
View file

@ -14,6 +14,7 @@ class Sequencer
'Import::Freshdesk::Ticket::Fetch',
'Import::Freshdesk::Ticket::Mapping',
'Import::Freshdesk::Mapping::CustomFields',
'Import::Common::Model::FindBy::Number',
'Import::Common::Model::Attributes::AddByIds',
'Import::Common::Model::Update',
'Import::Common::Model::Create',

1
lib/sequencer/sequence/import/freshdesk/time_entry.rb
View file

@ -10,6 +10,7 @@ class Sequencer
[
'Common::ModelClass::Ticket::TimeAccounting',
'Import::Freshdesk::TimeEntry::Mapping',
'Import::Common::Model::FindBy::TimeAccountingAttributes',
'Import::Common::Model::Update',
'Import::Common::Model::Create',
'Import::Common::Model::Save',

2
lib/sequencer/sequence/import/zendesk/ticket/comment.rb
View file

@ -17,7 +17,7 @@ class Sequencer
'Import::Zendesk::Ticket::Comment::Mapping',
'Import::Zendesk::Ticket::Comment::UnsetInstance',
'Common::ModelClass::Ticket::Article',
'Import::Common::Model::FindBy::Id',
'Import::Common::Model::FindBy::MessageId',
'Import::Common::Model::Update',
'Import::Common::Model::Create',
'Import::Common::Model::Save',

1
lib/sequencer/unit/import/common/model/create.rb
View file

@ -16,6 +16,7 @@ class Sequencer
def process
instance = model_class.new(mapped)
state.provide(:instance, instance)
state.provide(:action, :created)
rescue => e

16
lib/sequencer/unit/import/common/model/find_by/message_id.rb Normal file
View file

@ -0,0 +1,16 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
class Sequencer
class Unit
module Import
module Common
module Model
module FindBy
class MessageId < Sequencer::Unit::Import::Common::Model::FindBy::SameNamedAttribute
end
end
end
end
end
end
end

16
lib/sequencer/unit/import/common/model/find_by/number.rb Normal file
View file

@ -0,0 +1,16 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
class Sequencer
class Unit
module Import
module Common
module Model
module FindBy
class Number < Sequencer::Unit::Import::Common::Model::FindBy::SameNamedAttribute
end
end
end
end
end
end
end

22
lib/sequencer/unit/import/common/model/find_by/time_accounting_attributes.rb Normal file
View file

@ -0,0 +1,22 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
class Sequencer
class Unit
module Import
module Common
module Model
module FindBy
class TimeAccountingAttributes < Sequencer::Unit::Import::Common::Model::Lookup::CombinedAttributes
private
def attributes
%i[ticket_id created_by_id created_at]
end
end
end
end
end
end
end
end

32
lib/sequencer/unit/import/common/model/lookup/combined_attributes.rb Normal file
View file

@ -0,0 +1,32 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
class Sequencer
class Unit
module Import
module Common
module Model
module Lookup
class CombinedAttributes < Sequencer::Unit::Import::Common::Model::Lookup::Attributes
def existing_instance
@existing_instance ||= begin
filters = {}
Array(attributes).each do |attribute|
value = mapped[attribute]
next if value.blank?
filters[attribute] = value
end
if filters.present?
model_class.find_by(filters)
end
end
end
end
end
end
end
end
end
end

21
lib/sequencer/unit/import/freshdesk/request/ticket.rb
View file

@ -1,21 +0,0 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
class Sequencer
class Unit
module Import
module Freshdesk
class Request < Sequencer::Unit::Common::Provider::Attribute
class Ticket < Sequencer::Unit::Import::Freshdesk::Request::Generic
def params
super.merge(
updated_since: '1970-01-01',
order_type: :asc,
)
end
end
end
end
end
end
end

9
lib/sequencer/unit/import/freshdesk/resources.rb
View file

@ -7,12 +7,17 @@ class Sequencer
class Resources < Sequencer::Unit::Common::Provider::Named
include ::Sequencer::Unit::Import::Common::Model::Mixin::HandleFailure
uses :response
uses :response, :skipped_resource_id
private
def resources
JSON.parse(response.body)
body = JSON.parse(response.body)
return body if skipped_resource_id.nil?
# Remove the skipped resource id from the received resources.
body.reject { |item| item['id'] == skipped_resource_id }
rescue => e
logger.error "Won't be continued, because no response is available."
handle_failure(e)

46
lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb
View file

@ -9,23 +9,25 @@ class Sequencer
uses :dry_run, :import_job, :field_map, :id_map
attr_accessor :iteration
attr_accessor :iteration, :result
EXPECTING = %i[action response].freeze
def process
loop.each_with_index do |_, iteration|
@iteration = iteration
result = ::Sequencer.process(sequence_name,
parameters: {
request_params: request_params,
import_job: import_job,
dry_run: dry_run,
object: object,
field_map: field_map,
id_map: id_map,
},
expecting: %i[action response])
break if iteration_should_stop?(result)
@result = ::Sequencer.process(sequence_name,
parameters: {
request_params: request_params,
import_job: import_job,
dry_run: dry_run,
object: object,
field_map: field_map,
id_map: id_map,
skipped_resource_id: skipped_resource_id,
},
expecting: self.class.const_get(:EXPECTING))
break if iteration_should_stop?
end
end
@ -49,21 +51,15 @@ class Sequencer
private
def iteration_should_stop?(result)
def skipped_resource_id
@skipped_resource_id ||= nil
end
def iteration_should_stop?
return true if result[:action] == :failed
return true if result[:response].header['link'].blank?
max_page_reached?
end
# https://github.com/zammad/zammad/issues/3661
# https://developers.freshdesk.com/api/#list_all_tickets
def max_page_reached?
return false if object != 'Ticket'
return false if page <= 300
logger.warn "Reached max Freshdesk API page number #{page} for #{object}. Stopping further requests to prevent errors."
true
false
end
end
end

44
lib/sequencer/unit/import/freshdesk/tickets.rb
View file

@ -5,6 +5,50 @@ class Sequencer
module Import
module Freshdesk
class Tickets < Sequencer::Unit::Import::Freshdesk::SubSequence::Object
EXPECTING = %i[action response resources].freeze
private
def request_params
{
page: page,
updated_since: updated_since,
order_by: 'updated_at',
order_type: :asc,
}
end
def page
page_cycle + 1
end
def page_cycle
iteration % 300
end
def updated_since
@updated_since ||= '1970-01-01'
return @updated_since if !new_page_cycle?
@updated_since = result[:resources].last['updated_at']
end
def skipped_resource_id
super
return @skipped_resource_id if !new_page_cycle?
@skipped_resource_id = result[:resources].last['id']
end
def new_page_cycle?
return false if page_cycle != 0
return false if iteration.zero?
true
end
end
end
end

13
spec/lib/sequencer/sequence/import/freshdesk/conversation_spec.rb
View file

@ -76,14 +76,25 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Conversation, sequencer
end
end
it 'adds article with inline image' do # rubocop:disable RSpec/MultipleExpectations
it 'adds article with inline image' do
expect { process(process_payload) }.to change(Ticket::Article, :count).by(1)
end
it 'correct attributes for added article ' do
process(process_payload)
expect(Ticket::Article.last).to have_attributes(
to: 'info@zammad.org',
body: "\n<div>\n<div dir=\"ltr\">Let's see if inline images work in a subsequent article:</div>\n<div dir=\"ltr\"><img src=\"data:image/png;base64,MTIz\" style=\"width: auto;\"></div>\n</div>\n",
)
end
it 'updates already existing article' do
expect do
process(process_payload)
process(process_payload)
end.to change(Ticket::Article, :count).by(1)
end
it 'adds correct number of attachments' do
process(process_payload)
expect(Ticket::Article.last.attachments.size).to eq 1

13
spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb
View file

@ -33,12 +33,13 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::GenericObject, sequence
let(:process_payload) do
{
import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}),
dry_run: false,
object: 'Group',
request_params: {},
field_map: {},
id_map: {},
import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}),
dry_run: false,
object: 'Group',
request_params: {},
field_map: {},
id_map: {},
skipped_resource_id: nil,
}
end

112
spec/lib/sequencer/sequence/import/freshdesk/ticket_spec.rb
View file

@ -42,7 +42,7 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Ticket, sequencer: :seq
'created_at' => '2021-05-14T12:29:27Z',
'updated_at' => '2021-05-14T12:30:19Z',
'associated_tickets_count' => nil,
'tags' => [],
'tags' => %w[example test],
'description' => "<div style=\"font-family:-apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Helvetica Neue, Arial, sans-serif; font-size:14px\">\n<div dir=\"ltr\">Inline images in the first article might not be working, see following:</div>\n<div dir=\"ltr\"><img src=\"https://eucattachment.freshdesk.com/inline/attachment?token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ODAwMTIyMjY0NzksImRvbWFpbiI6InphbW1hZC5mcmVzaGRlc2suY29tIiwiYWNjb3VudF9pZCI6MTg5MDU2MH0.cdYIOOSi7ckCFIZlQ9eynELMzJp1ECVeTLlQMCDgKo4\" style=\"width: auto\" class=\"fr-fil fr-dib\" data-id=\"80012226479\"></div>\n</div>", 'description_text' => 'Inline images in the first article might not be working, see following:'
}
@ -103,31 +103,8 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Ticket, sequencer: :seq
]
end
before do
create :object_manager_attribute_select, name: 'cf_custom_dropdown'
create :object_manager_attribute_integer, name: 'cf_custom_integer'
create :object_manager_attribute_boolean, name: 'cf_test_checkbox'
create :object_manager_attribute_text, name: 'cf_custom_decimal'
ObjectManager::Attribute.migration_execute
# Mock the attachment and inline image download requests.
used_urls.each do |used_url|
stub_request(:get, used_url).to_return(status: 200, body: '123', headers: {})
end
# Mock the ticket get request (Import::Freshdesk::Ticket::Fetch).
stub_request(:get, 'https://yours.freshdesk.com/api/v2/tickets/13').to_return(status: 200, body: JSON.generate(ticket_get_response_payload), headers: {})
end
# We only want to test here the Ticket API, so disable other modules in the sequence
# that make their own HTTP requests.
custom_sequence = Sequencer::Sequence::Import::Freshdesk::Ticket.sequence.dup
custom_sequence.delete('Import::Freshdesk::Ticket::TimeEntries')
custom_sequence.delete('Import::Freshdesk::Ticket::Conversations')
it 'adds tickets' do # rubocop:disable RSpec/MultipleExpectations, RSpec/ExampleLength
allow(Sequencer::Sequence::Import::Freshdesk::Ticket).to receive(:sequence) { custom_sequence }
expect { process(process_payload) }.to change(Ticket, :count).by(1)
expect(Ticket.last).to have_attributes(
let(:imported_ticket) do
{
title: 'Inline Images Failing?',
note: nil,
create_article_type_id: 5,
@ -142,12 +119,63 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Ticket, sequencer: :seq
cf_custom_integer: 999,
cf_test_checkbox: true,
cf_custom_decimal: '1.1',
)
}
end
it 'adds article with inline image' do # rubocop:disable RSpec/MultipleExpectations, RSpec/ExampleLength
let(:imported_article_attachments) do
{
'filename' => 'standalone_attachment.png',
'size' => '3',
'preferences' => {
'Content-Type' => 'image/png',
'resizable' => false,
}
}
end
before do
create :object_manager_attribute_select, name: 'cf_custom_dropdown'
create :object_manager_attribute_integer, name: 'cf_custom_integer'
create :object_manager_attribute_boolean, name: 'cf_test_checkbox'
create :object_manager_attribute_text, name: 'cf_custom_decimal'
ObjectManager::Attribute.migration_execute
# Mock the attachment and inline image download requests.
used_urls.each do |used_url|
stub_request(:get, used_url).to_return(status: 200, body: '123', headers: {})
end
# Mock the ticket get request (Import::Freshdesk::Ticket::Fetch).
stub_request(:get, 'https://yours.freshdesk.com/api/v2/tickets/13').to_return(status: 200, body: JSON.generate(ticket_get_response_payload), headers: {})
# We only want to test here the Ticket API, so disable other modules in the sequence
# that make their own HTTP requests.
custom_sequence = Sequencer::Sequence::Import::Freshdesk::Ticket.sequence.dup
custom_sequence.delete('Import::Freshdesk::Ticket::TimeEntries')
custom_sequence.delete('Import::Freshdesk::Ticket::Conversations')
allow(Sequencer::Sequence::Import::Freshdesk::Ticket).to receive(:sequence) { custom_sequence }
end
it 'adds tickets' do
expect { process(process_payload) }.to change(Ticket, :count).by(1)
end
it 'correct attributes for added ticket' do
process(process_payload)
expect(Ticket.last).to have_attributes(imported_ticket)
end
it 'correct tags for added ticket' do
process(process_payload)
expect(Ticket.last.tag_list).to eq(%w[example test])
end
it 'adds article with inline image' do
expect { process(process_payload) }.to change(Ticket::Article, :count).by(1)
end
it 'correct attributes for added article ' do
process(process_payload)
expect(Ticket::Article.last).to have_attributes(
to: 'info@zammad.org',
body: "\n<div>\n<div dir=\"ltr\">Inline images in the first article might not be working, see following:</div>\n<div dir=\"ltr\"><img src=\"data:image/png;base64,MTIz\" style=\"width: auto;\"></div>\n</div>\n",
@ -155,22 +183,28 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Ticket, sequencer: :seq
end
it 'adds correct number of attachments' do
allow(Sequencer::Sequence::Import::Freshdesk::Ticket).to receive(:sequence) { custom_sequence }
process(process_payload)
expect(Ticket::Article.last.attachments.size).to eq 1
end
it 'adds attachment content' do # rubocop:disable RSpec/ExampleLength
allow(Sequencer::Sequence::Import::Freshdesk::Ticket).to receive(:sequence) { custom_sequence }
it 'adds attachment content' do
process(process_payload)
expect(Ticket::Article.last.attachments.last).to have_attributes(
'filename' => 'standalone_attachment.png',
'size' => '3',
'preferences' => {
'Content-Type' => 'image/png',
'resizable' => false,
}
)
expect(Ticket::Article.last.attachments.last).to have_attributes(imported_article_attachments)
end
context 'when ticket is imported twice' do
before do
process(process_payload)
end
it 'updates first article for already existing ticket' do
expect { process(process_payload) }.to change(Ticket::Article, :count).by(0)
end
it 'correct tags for added ticket' do
process(process_payload)
expect(Ticket.last.tag_list).to eq(%w[example test])
end
end
end
end

70
spec/lib/sequencer/sequence/import/freshdesk/time_entry_spec.rb Normal file
View file

@ -0,0 +1,70 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
require 'rails_helper'
RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::TimeEntry, sequencer: :sequence do
context 'when importing time_entry from Freshdesk' do
let(:resource) do
{
'id' => 80_027_218_656,
'billable' => true,
'note' => 'Example Prepartion',
'timer_running' => false,
'agent_id' => 80_014_400_475,
'ticket_id' => 1001,
'time_spent' => '01:20',
'created_at' => '2021-05-14T12:29:27Z',
'updated_at' => '2021-05-14T12:29:27Z',
'start_time' => '2021-05-14T12:29:27Z',
'executed_at' => '2021-05-14T12:29:27Z'
}
end
let(:ticket) { create :ticket }
let(:id_map) do
{
'Ticket' => {
1001 => ticket.id,
},
'User' => {
80_014_400_475 => 1,
}
}
end
let(:process_payload) do
{
import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}),
dry_run: false,
resource: resource,
field_map: {},
id_map: id_map,
}
end
let(:imported_time_entry) do
{
ticket_id: ticket.id,
created_by_id: 1,
time_unit: 80,
}
end
it 'adds time entry' do
expect { process(process_payload) }.to change(Ticket::TimeAccounting, :count).by(1)
end
it 'correct attributes for added time entry ' do
process(process_payload)
expect(Ticket::TimeAccounting.last).to have_attributes(imported_time_entry)
end
it 'updates already existing article' do
expect do
process(process_payload)
process(process_payload)
end.to change(Ticket::TimeAccounting, :count).by(1)
end
end
end

136
spec/lib/sequencer/unit/import/freshdesk/tickets_spec.rb Normal file
View file

@ -0,0 +1,136 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
require 'rails_helper'
RSpec.describe ::Sequencer::Unit::Import::Freshdesk::Tickets, sequencer: :unit, db_strategy: 'reset' do
context 'when importing ticket list from freshdesk' do
let(:group) { create :group }
let(:owner) { create :agent, group_ids: [group.id] }
let(:id_map) do
{
'User' => {
80_014_400_475 => owner.id,
},
'Group' => {
80_000_374_718 => group.id,
},
}
end
let(:ticket_data) do
{
'cc_emails' => [],
'fwd_emails' => [],
'reply_cc_emails' => [],
'ticket_cc_emails' => [],
'fr_escalated' => false,
'spam' => false,
'email_config_id' => nil,
'group_id' => 80_000_374_718,
'priority' => 1,
'requester_id' => 80_014_400_475,
'responder_id' => 80_014_400_475,
'source' => 3,
'company_id' => nil,
'status' => 2,
'subject' => 'Inline Images Failing?',
'association_type' => nil,
'support_email' => nil,
'to_emails' => ['info@zammad.org'],
'product_id' => nil,
'type' => nil,
'due_by' => '2021-05-17T12:29:27Z',
'fr_due_by' => '2021-05-15T12:29:27Z',
'is_escalated' => false,
'custom_fields' => {},
'created_at' => '2021-05-14T12:29:27Z',
'updated_at' => '2021-05-14T12:30:19Z',
'associated_tickets_count' => nil,
'tags' => [],
}
end
let(:resources_payloud) do
[
ticket_data.merge(
id: 10
),
ticket_data.merge(
id: 11
),
]
end
let(:process_payload) do
{
import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}),
dry_run: false,
request_params: {},
field_map: {},
id_map: id_map,
}
end
before do
# We only want to test here the Ticket API, so disable other modules in the sequence
# that make their own HTTP requests.
custom_sequence = Sequencer::Sequence::Import::Freshdesk::Ticket.sequence.dup
custom_sequence.delete('Import::Freshdesk::Ticket::Fetch')
custom_sequence.delete('Import::Freshdesk::Ticket::TimeEntries')
custom_sequence.delete('Import::Freshdesk::Ticket::Conversations')
allow(Sequencer::Sequence::Import::Freshdesk::Ticket).to receive(:sequence) { custom_sequence }
# Mock the groups get request
stub_request(:get, 'https://yours.freshdesk.com/api/v2/tickets?order_by=updated_at&order_type=asc&page=1&per_page=100&updated_since=1970-01-01').to_return(status: 200, body: JSON.generate(resources_payloud), headers: {})
end
context 'when page limit is not reached' do
it 'check ticket count' do
expect { process(process_payload) }.to change(Ticket, :count).by(2)
end
end
context 'when page limit is reached' do
let(:resources_payloud_second_cycle) do
[
ticket_data.merge(
id: 11
),
ticket_data.merge(
id: 12
),
# Subject was changed during the import.
ticket_data.merge(
id: 10,
subject: 'Different subject'
),
]
end
before do
stub_request(:get, 'https://yours.freshdesk.com/api/v2/tickets?order_by=updated_at&order_type=asc&page=1&per_page=100&updated_since=1970-01-01').to_return(status: 200, body: JSON.generate(resources_payloud), headers: { link: 'second-page' })
stub_request(:get, 'https://yours.freshdesk.com/api/v2/tickets?order_by=updated_at&order_type=asc&page=1&per_page=100&&updated_since=2021-05-14T12:30:19Z').to_return(status: 200, body: JSON.generate(resources_payloud_second_cycle), headers: {})
end
it 'check ticket count' do
expect do
process(process_payload) do |instance|
# Set the `page_cycle` to zero, so that we trigger a new cycle (normally if more then 30.000 tickets exists)
allow(instance).to receive(:page_cycle).and_return(0)
end
end.to change(Ticket, :count).by(3)
end
it 'check that ticket title was changed during import' do
process(process_payload) do |instance|
# Set the `page_cycle` to zero, so that we trigger a new cycle (normally if more then 30.000 tickets exists)
allow(instance).to receive(:page_cycle).and_return(0)
end
expect(Ticket.find_by(number: 10).title).to eq('Different subject')
end
end
end
end

2827
test/data/vcr_cassettes/freshdesk_import.yml
View file

File diff suppressed because one or more lines are too long