From 3cada0f15a476c5b057560fc22de07210394fafb Mon Sep 17 00:00:00 2001 From: Dominik Klein Date: Thu, 14 Apr 2022 07:43:04 +0200 Subject: [PATCH] Fixes #4052 - Freshdesk migration which doesn't support time entries runs in unwanted behaviour. --- .../sequence/import/freshdesk/full.rb | 1 + .../sequence/import/freshdesk/time_entries.rb | 1 + .../unit/import/freshdesk/model_class.rb | 3 + .../unit/import/freshdesk/perform.rb | 13 ++- .../unit/import/freshdesk/request.rb | 4 +- .../unit/import/freshdesk/requester.rb | 2 + .../unit/import/freshdesk/resources.rb | 3 + .../import/freshdesk/sub_sequence/generic.rb | 19 ++-- .../import/freshdesk/ticket/time_entries.rb | 2 - .../import/freshdesk/time_entry/available.rb | 34 ++++++ .../import/freshdesk/time_entry/mapping.rb | 9 -- .../unit/import/freshdesk/time_entry/skip.rb | 22 ++++ .../import/freshdesk/generic_object_spec.rb | 15 +-- .../sequence/import/freshdesk/ticket_spec.rb | 11 +- .../import/freshdesk/time_entries_spec.rb | 103 ++++++++++++++++++ .../import/freshdesk/time_entry_spec.rb | 3 +- .../unit/import/freshdesk/tickets_spec.rb | 11 +- .../freshdesk/time_entry/available_spec.rb | 31 ++++++ 18 files changed, 242 insertions(+), 45 deletions(-) create mode 100644 lib/sequencer/unit/import/freshdesk/time_entry/available.rb create mode 100644 lib/sequencer/unit/import/freshdesk/time_entry/skip.rb create mode 100644 spec/lib/sequencer/sequence/import/freshdesk/time_entries_spec.rb create mode 100644 spec/lib/sequencer/unit/import/freshdesk/time_entry/available_spec.rb diff --git a/lib/sequencer/sequence/import/freshdesk/full.rb b/lib/sequencer/sequence/import/freshdesk/full.rb index 3df520a03..0677781c4 100644 --- a/lib/sequencer/sequence/import/freshdesk/full.rb +++ b/lib/sequencer/sequence/import/freshdesk/full.rb @@ -11,6 +11,7 @@ class Sequencer 'Import::Common::ImportMode::Check', 'Import::Common::SystemInitDone::Check', 'Import::Common::ImportJob::DryRun', + 'Import::Freshdesk::TimeEntry::Available', 'Import::Freshdesk::IdMap', 'Import::Freshdesk::Groups', 'Import::Freshdesk::FieldMap', diff --git a/lib/sequencer/sequence/import/freshdesk/time_entries.rb b/lib/sequencer/sequence/import/freshdesk/time_entries.rb index c3219816e..7e956f45b 100644 --- a/lib/sequencer/sequence/import/freshdesk/time_entries.rb +++ b/lib/sequencer/sequence/import/freshdesk/time_entries.rb @@ -8,6 +8,7 @@ class Sequencer def self.sequence [ + 'Import::Freshdesk::TimeEntry::Skip', 'Import::Freshdesk::Request', 'Import::Freshdesk::Resources', 'Import::Freshdesk::ModelClass', diff --git a/lib/sequencer/unit/import/freshdesk/model_class.rb b/lib/sequencer/unit/import/freshdesk/model_class.rb index 253ef0cf3..4421751fa 100644 --- a/lib/sequencer/unit/import/freshdesk/model_class.rb +++ b/lib/sequencer/unit/import/freshdesk/model_class.rb @@ -5,6 +5,9 @@ class Sequencer module Import module Freshdesk class ModelClass < Sequencer::Unit::Common::Provider::Named + prepend ::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action + + skip_action :skipped, :failed uses :object diff --git a/lib/sequencer/unit/import/freshdesk/perform.rb b/lib/sequencer/unit/import/freshdesk/perform.rb index c68dda209..2fe974789 100644 --- a/lib/sequencer/unit/import/freshdesk/perform.rb +++ b/lib/sequencer/unit/import/freshdesk/perform.rb @@ -9,17 +9,18 @@ class Sequencer skip_action :skipped, :failed - uses :resources, :object, :import_job, :dry_run, :field_map, :id_map + uses :resources, :object, :import_job, :dry_run, :field_map, :id_map, :time_entry_available def process resources.each do |resource| ::Sequencer.process("Import::Freshdesk::#{object}", parameters: { - import_job: import_job, - dry_run: dry_run, - resource: resource, - field_map: field_map, - id_map: id_map, + import_job: import_job, + dry_run: dry_run, + resource: resource, + field_map: field_map, + id_map: id_map, + time_entry_available: time_entry_available, }) end end diff --git a/lib/sequencer/unit/import/freshdesk/request.rb b/lib/sequencer/unit/import/freshdesk/request.rb index 486a56604..928f59b27 100644 --- a/lib/sequencer/unit/import/freshdesk/request.rb +++ b/lib/sequencer/unit/import/freshdesk/request.rb @@ -6,6 +6,9 @@ class Sequencer module Freshdesk class Request < Sequencer::Unit::Common::Provider::Attribute extend ::Sequencer::Unit::Import::Freshdesk::Requester + prepend ::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action + + skip_action :skipped, :failed uses :object, :request_params provides :response @@ -13,7 +16,6 @@ class Sequencer private def response - builder = backend.new( object: object, request_params: request_params diff --git a/lib/sequencer/unit/import/freshdesk/requester.rb b/lib/sequencer/unit/import/freshdesk/requester.rb index 6c273ec94..6f9d72485 100644 --- a/lib/sequencer/unit/import/freshdesk/requester.rb +++ b/lib/sequencer/unit/import/freshdesk/requester.rb @@ -31,12 +31,14 @@ class Sequencer else logger.info "Unknown response: #{response.inspect}. Sleeping 10 seconds and retry (##{iteration + 1}/10)." end + sleep sleep_for end def handle_exception(e, iteration) logger.error e logger.info "Sleeping 10 seconds after #{e.class.name} and retry (##{iteration + 1}/10)." + sleep 10 end diff --git a/lib/sequencer/unit/import/freshdesk/resources.rb b/lib/sequencer/unit/import/freshdesk/resources.rb index 2ab72e4db..b3bbeb8f4 100644 --- a/lib/sequencer/unit/import/freshdesk/resources.rb +++ b/lib/sequencer/unit/import/freshdesk/resources.rb @@ -6,6 +6,9 @@ class Sequencer module Freshdesk class Resources < Sequencer::Unit::Common::Provider::Named include ::Sequencer::Unit::Import::Common::Model::Mixin::HandleFailure + prepend ::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action + + skip_action :skipped, :failed uses :response, :skipped_resource_id diff --git a/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb b/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb index fd5fe16ed..60b18ccc8 100644 --- a/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb +++ b/lib/sequencer/unit/import/freshdesk/sub_sequence/generic.rb @@ -7,7 +7,7 @@ class Sequencer module SubSequence class Generic < Sequencer::Unit::Base - uses :dry_run, :import_job, :field_map, :id_map + uses :dry_run, :import_job, :field_map, :id_map, :time_entry_available attr_accessor :iteration, :result @@ -18,13 +18,14 @@ class Sequencer @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, - skipped_resource_id: skipped_resource_id, + 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, + time_entry_available: time_entry_available, }, expecting: self.class.const_get(:EXPECTING)) break if iteration_should_stop? @@ -56,7 +57,7 @@ class Sequencer end def iteration_should_stop? - return true if result[:action] == :failed + return true if result[:action] == :failed || result[:action] == :skipped return true if result[:response].header['link'].blank? false diff --git a/lib/sequencer/unit/import/freshdesk/ticket/time_entries.rb b/lib/sequencer/unit/import/freshdesk/ticket/time_entries.rb index 13a78f7a9..d612aa63d 100644 --- a/lib/sequencer/unit/import/freshdesk/ticket/time_entries.rb +++ b/lib/sequencer/unit/import/freshdesk/ticket/time_entries.rb @@ -8,8 +8,6 @@ class Sequencer class TimeEntries < Sequencer::Unit::Import::Freshdesk::SubSequence::Generic prepend ::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action - optional :action - skip_action :skipped, :failed uses :resource diff --git a/lib/sequencer/unit/import/freshdesk/time_entry/available.rb b/lib/sequencer/unit/import/freshdesk/time_entry/available.rb new file mode 100644 index 000000000..d1f5864cc --- /dev/null +++ b/lib/sequencer/unit/import/freshdesk/time_entry/available.rb @@ -0,0 +1,34 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class Sequencer + class Unit + module Import + module Freshdesk + module TimeEntry + class Available < Sequencer::Unit::Common::Provider::Attribute + extend ::Sequencer::Unit::Import::Freshdesk::Requester + + provides :time_entry_available + + def process + state.provide(:time_entry_available, time_entry_available) + end + + private + + def time_entry_available + response = self.class.perform_request( + api_path: 'time_entries', + ) + + response.is_a?(Net::HTTPOK) + rescue => e + logger.info e + nil + end + end + end + end + end + end +end diff --git a/lib/sequencer/unit/import/freshdesk/time_entry/mapping.rb b/lib/sequencer/unit/import/freshdesk/time_entry/mapping.rb index f5709c9a9..4c24d6616 100644 --- a/lib/sequencer/unit/import/freshdesk/time_entry/mapping.rb +++ b/lib/sequencer/unit/import/freshdesk/time_entry/mapping.rb @@ -9,7 +9,6 @@ class Sequencer include ::Sequencer::Unit::Import::Common::Mapping::Mixin::ProvideMapped uses :resource, :id_map - provides :action def process provide_mapped do @@ -21,14 +20,6 @@ class Sequencer updated_at: resource['updated_at'], } end - rescue TypeError => e - # TimeTracking is not available in the plans: Sprout, Blossom - # In this case `resource`s value is `["code", "require_feature"]` - # See: - # - Ticket# 1077135 - # - https://support.freshdesk.com/support/solutions/articles/37583-keeping-track-of-time-spent - logger.debug { e } - state.provide(:action, :skipped) end private diff --git a/lib/sequencer/unit/import/freshdesk/time_entry/skip.rb b/lib/sequencer/unit/import/freshdesk/time_entry/skip.rb new file mode 100644 index 000000000..bf96877bb --- /dev/null +++ b/lib/sequencer/unit/import/freshdesk/time_entry/skip.rb @@ -0,0 +1,22 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +class Sequencer + class Unit + module Import + module Freshdesk + module TimeEntry + class Skip < Sequencer::Unit::Base + uses :time_entry_available + provides :action + + def process + return if time_entry_available + + state.provide(:action, :skipped) + end + end + end + 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 index 0fdf31801..1898955f8 100644 --- a/spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb +++ b/spec/lib/sequencer/sequence/import/freshdesk/generic_object_spec.rb @@ -33,13 +33,14 @@ 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: {}, - skipped_resource_id: nil, + 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, + time_entry_available: true, } end diff --git a/spec/lib/sequencer/sequence/import/freshdesk/ticket_spec.rb b/spec/lib/sequencer/sequence/import/freshdesk/ticket_spec.rb index 8c0d07d76..abed040ef 100644 --- a/spec/lib/sequencer/sequence/import/freshdesk/ticket_spec.rb +++ b/spec/lib/sequencer/sequence/import/freshdesk/ticket_spec.rb @@ -69,11 +69,12 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Ticket, sequencer: :seq end let(:process_payload) do { - import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}), - dry_run: false, - resource: resource, - field_map: field_map, - id_map: id_map, + import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}), + dry_run: false, + resource: resource, + field_map: field_map, + id_map: id_map, + time_entry_available: true, } end let(:owner) { create :agent, group_ids: [group.id] } diff --git a/spec/lib/sequencer/sequence/import/freshdesk/time_entries_spec.rb b/spec/lib/sequencer/sequence/import/freshdesk/time_entries_spec.rb new file mode 100644 index 000000000..a43a02cfa --- /dev/null +++ b/spec/lib/sequencer/sequence/import/freshdesk/time_entries_spec.rb @@ -0,0 +1,103 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::TimeEntries, sequencer: :sequence, db_strategy: 'reset' do + let(:time_entry_available) { true } + let(:ticket) { create :ticket } + + let(:process_payload) do + { + import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}), + dry_run: false, + object: 'TimeEntry', + request_params: { + ticket: { + 'id' => 1001, + }, + }, + field_map: {}, + id_map: { + 'Ticket' => { + 1001 => ticket.id, + }, + 'User' => { + 80_014_400_475 => 1, + } + }, + skipped_resource_id: nil, + time_entry_available: time_entry_available, + } + end + + context 'when time entry feature is available' do + let(:resources_payloud) do + [ + { + 'id' => 80_027_218_656, + 'billable' => true, + 'note' => 'Example Preparation', + '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' + }, + { + 'id' => 80_027_218_657, + 'billable' => true, + 'note' => 'Example Preparation 2', + 'timer_running' => false, + 'agent_id' => 80_014_400_475, + 'ticket_id' => 1001, + 'time_spent' => '02:20', + 'created_at' => '2021-05-15T12:29:27Z', + 'updated_at' => '2021-05-15T12:29:27Z', + 'start_time' => '2021-05-15T12:29:27Z', + 'executed_at' => '2021-05-15T12:29:27Z' + } + ] + end + + let(:imported_time_entry) do + { + ticket_id: ticket.id, + created_by_id: 1, + time_unit: 140, + } + end + + before do + # Mock the groups get request + stub_request(:get, 'https://yours.freshdesk.com/api/v2/tickets/1001/time_entries?per_page=100').to_return(status: 200, body: JSON.generate(resources_payloud), headers: {}) + end + + it 'add time entry for ticket' do + expect { process(process_payload) }.to change(Ticket::TimeAccounting, :count).by(2) + end + + it 'check last time unit for ticket' do + process(process_payload) + expect(Ticket::TimeAccounting.last).to have_attributes(imported_time_entry) + end + + context 'with empty time entries' do + let(:resources_payloud) { [] } + + it 'do not change time entry for ticket' do + expect { process(process_payload) }.to change(Ticket::TimeAccounting, :count).by(0) + end + end + end + + context 'when time entry feature is not available' do + let(:time_entry_available) { false } + + it 'add time entry for ticket' do + expect { process(process_payload) }.to change(Ticket::TimeAccounting, :count).by(0) + end + end +end diff --git a/spec/lib/sequencer/sequence/import/freshdesk/time_entry_spec.rb b/spec/lib/sequencer/sequence/import/freshdesk/time_entry_spec.rb index 89aa2305a..8ecb83e4b 100644 --- a/spec/lib/sequencer/sequence/import/freshdesk/time_entry_spec.rb +++ b/spec/lib/sequencer/sequence/import/freshdesk/time_entry_spec.rb @@ -10,7 +10,7 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::TimeEntry, sequencer: : { 'id' => 80_027_218_656, 'billable' => true, - 'note' => 'Example Prepartion', + 'note' => 'Example Preparation', 'timer_running' => false, 'agent_id' => 80_014_400_475, 'ticket_id' => 1001, @@ -57,6 +57,7 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::TimeEntry, sequencer: : it 'correct attributes for added time entry' do process(process_payload) + Rails.logger.debug Ticket::TimeAccounting.last expect(Ticket::TimeAccounting.last).to have_attributes(imported_time_entry) end diff --git a/spec/lib/sequencer/unit/import/freshdesk/tickets_spec.rb b/spec/lib/sequencer/unit/import/freshdesk/tickets_spec.rb index 95b7c80d2..45bfb1812 100644 --- a/spec/lib/sequencer/unit/import/freshdesk/tickets_spec.rb +++ b/spec/lib/sequencer/unit/import/freshdesk/tickets_spec.rb @@ -64,11 +64,12 @@ RSpec.describe ::Sequencer::Unit::Import::Freshdesk::Tickets, sequencer: :unit, 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, + import_job: build_stubbed(:import_job, name: 'Import::Freshdesk', payload: {}), + dry_run: false, + request_params: {}, + field_map: {}, + id_map: id_map, + time_entry_available: true, } end diff --git a/spec/lib/sequencer/unit/import/freshdesk/time_entry/available_spec.rb b/spec/lib/sequencer/unit/import/freshdesk/time_entry/available_spec.rb new file mode 100644 index 000000000..eaf741314 --- /dev/null +++ b/spec/lib/sequencer/unit/import/freshdesk/time_entry/available_spec.rb @@ -0,0 +1,31 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe Sequencer::Unit::Import::Freshdesk::TimeEntry::Available, sequencer: :unit do + + context 'when checking the availability of the time entry feature' do + + let(:params) do + { + dry_run: false, + import_job: instance_double(ImportJob), + field_map: {}, + id_map: {}, + } + end + + let(:response_ok) { Net::HTTPOK.new(1.0, '200', 'OK') } + let(:response_forbidden) { Net::HTTPUnauthorized.new(1.0, '403', 'Forbidden') } + + it 'check for avilable time entry feature' do + allow(described_class).to receive(:perform_request).with(any_args).and_return(response_ok) + expect(process(params)).to eq({ time_entry_available: true }) + end + + it 'check for not available time entry feature' do + allow(described_class).to receive(:perform_request).with(any_args).and_return(response_forbidden) + expect(process(params)).to eq({ time_entry_available: false }) + end + end +end