From 0ded6e542bfcf43a8ee1d3a4161f53acb1f44086 Mon Sep 17 00:00:00 2001 From: Dominik Klein Date: Wed, 28 Jul 2021 16:40:08 +0200 Subject: [PATCH] Fixes #3661 - FreshDesk Import Error - undefined method `body' for 10:Integer. --- .../import/freshdesk/conversations.rb | 2 +- .../sequence/import/freshdesk/time_entries.rb | 2 +- .../common/import_job/statistics/update.rb | 2 + .../unit/import/freshdesk/object_count.rb | 3 + .../unit/import/freshdesk/perform.rb | 3 + .../unit/import/freshdesk/requester.rb | 2 + .../unit/import/freshdesk/resources.rb | 4 ++ .../import/freshdesk/sub_sequence/generic.rb | 31 +++++++-- .../import/freshdesk/generic_object_spec.rb | 65 +++++++++++++++++++ 9 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb diff --git a/lib/sequencer/sequence/import/freshdesk/conversations.rb b/lib/sequencer/sequence/import/freshdesk/conversations.rb index 257d30f22..f2f456207 100644 --- a/lib/sequencer/sequence/import/freshdesk/conversations.rb +++ b/lib/sequencer/sequence/import/freshdesk/conversations.rb @@ -8,7 +8,7 @@ class Sequencer def self.sequence [ - 'Sequencer::Unit::Import::Freshdesk::Request', + 'Import::Freshdesk::Request', 'Import::Freshdesk::Resources', 'Import::Freshdesk::ModelClass', 'Import::Freshdesk::Perform', diff --git a/lib/sequencer/sequence/import/freshdesk/time_entries.rb b/lib/sequencer/sequence/import/freshdesk/time_entries.rb index 105111c71..100624666 100644 --- a/lib/sequencer/sequence/import/freshdesk/time_entries.rb +++ b/lib/sequencer/sequence/import/freshdesk/time_entries.rb @@ -8,7 +8,7 @@ class Sequencer def self.sequence [ - 'Sequencer::Unit::Import::Freshdesk::Request', + 'Import::Freshdesk::Request', 'Import::Freshdesk::Resources', 'Import::Freshdesk::ModelClass', 'Import::Freshdesk::Perform', diff --git a/lib/sequencer/unit/import/common/import_job/statistics/update.rb b/lib/sequencer/unit/import/common/import_job/statistics/update.rb index 00ec51498..003d04483 100644 --- a/lib/sequencer/unit/import/common/import_job/statistics/update.rb +++ b/lib/sequencer/unit/import/common/import_job/statistics/update.rb @@ -33,6 +33,8 @@ class Sequencer end def sum_deeply(existing:, additions:) + return existing if additions.nil? + existing.merge(additions) do |_key, oldval, newval| if oldval.is_a?(Hash) || newval.is_a?(Hash) sum_deeply( diff --git a/lib/sequencer/unit/import/freshdesk/object_count.rb b/lib/sequencer/unit/import/freshdesk/object_count.rb index 365719dd1..cdbe741dd 100644 --- a/lib/sequencer/unit/import/freshdesk/object_count.rb +++ b/lib/sequencer/unit/import/freshdesk/object_count.rb @@ -6,6 +6,9 @@ class Sequencer module Freshdesk class ObjectCount < Sequencer::Unit::Common::Provider::Attribute include ::Sequencer::Unit::Import::Common::Model::Statistics::Mixin::EmptyDiff + prepend ::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action + + skip_action :skipped, :failed uses :model_class, :resources diff --git a/lib/sequencer/unit/import/freshdesk/perform.rb b/lib/sequencer/unit/import/freshdesk/perform.rb index b7c60c42c..01098da09 100644 --- a/lib/sequencer/unit/import/freshdesk/perform.rb +++ b/lib/sequencer/unit/import/freshdesk/perform.rb @@ -5,6 +5,9 @@ class Sequencer module Import module Freshdesk class Perform < Sequencer::Unit::Base + prepend ::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action + + skip_action :skipped, :failed uses :resources, :object, :import_job, :dry_run, :field_map, :id_map diff --git a/lib/sequencer/unit/import/freshdesk/requester.rb b/lib/sequencer/unit/import/freshdesk/requester.rb index 9a44e871f..94f024630 100644 --- a/lib/sequencer/unit/import/freshdesk/requester.rb +++ b/lib/sequencer/unit/import/freshdesk/requester.rb @@ -18,6 +18,8 @@ class Sequencer rescue Net::HTTPClientError => e handle_exception e, iteration end + + nil end def handle_error(response, iteration) diff --git a/lib/sequencer/unit/import/freshdesk/resources.rb b/lib/sequencer/unit/import/freshdesk/resources.rb index ca6fd0645..2e287dbca 100644 --- a/lib/sequencer/unit/import/freshdesk/resources.rb +++ b/lib/sequencer/unit/import/freshdesk/resources.rb @@ -5,6 +5,7 @@ class Sequencer module Import module Freshdesk class Resources < Sequencer::Unit::Common::Provider::Named + include ::Sequencer::Unit::Import::Common::Model::Mixin::HandleFailure uses :response @@ -12,6 +13,9 @@ class Sequencer def resources JSON.parse(response.body) + rescue => e + logger.error "Won't be continued, because no response is available." + handle_failure(e) end end end diff --git a/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb b/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb index a3da41166..df526bc72 100644 --- a/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb +++ b/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb @@ -24,24 +24,47 @@ class Sequencer field_map: field_map, id_map: id_map, }, - expecting: [:response]) - break if result[:response].header['link'].blank? + expecting: %i[action response]) + break if iteration_should_stop?(result) end end def request_params { - page: iteration + 1, + page: page, } end + def page + iteration + 1 + end + def object - self.class.name.demodulize.singularize + @object ||= self.class.name.demodulize.singularize end def sequence_name raise NotImplementedError end + + private + + def iteration_should_stop?(result) + 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 + end end end end diff --git a/spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb b/spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb new file mode 100644 index 000000000..6acc11dac --- /dev/null +++ b/spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb @@ -0,0 +1,65 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::GenericObject, sequencer: :sequence, db_strategy: 'reset' do + context 'when importing group list with generic object' do + let(:resources_payloud) do + [ + { + 'id' => 80_000_374_715, + 'name' => 'QA', + 'description' => 'Members of the QA team belong to this group', + 'escalate_to' => nil, + 'unassigned_for' => nil, + 'business_hour_id' => nil, + 'group_type' => 'support_agent_group', + 'created_at' => '2021-04-09T13:23:59Z', + 'updated_at' => '2021-04-09T13:23:59Z' + }, + { + 'id' => 80_000_374_716, + 'name' => 'Testing', + 'description' => 'Members of the Testing team belong to this group', + 'escalate_to' => nil, + 'unassigned_for' => nil, + 'business_hour_id' => nil, + 'group_type' => 'support_agent_group', + 'created_at' => '2021-04-09T13:23:59Z', + 'updated_at' => '2021-04-09T13:23:59Z' + } + ] + end + + 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: {}, + } + end + + before do + # Mock the groups get request + stub_request(:get, 'https://yours.freshdesk.com/api/v2/groups?per_page=100').to_return(status: 200, body: JSON.generate(resources_payloud), headers: {}) + end + + it 'add groups' do + expect { process(process_payload) }.to change(Group, :count).by(2) + end + + context 'when list request fails' do + before do + allow(Sequencer::Unit::Import::Freshdesk::Request).to receive(:handle_error).with(any_args).and_return(true) + stub_request(:get, 'https://yours.freshdesk.com/api/v2/groups?per_page=100').to_return(status: 400, headers: {}) + end + + it 'check that a failing response do not raise a hard error' do + expect { process(process_payload) }.to change(Group, :count).by(0) + end + end + end +end