From 61f42f950f35d9fc93bd6b13ec7810b8faff1f11 Mon Sep 17 00:00:00 2001 From: Dominik Klein Date: Tue, 10 May 2022 12:21:53 +0200 Subject: [PATCH] Fixes #4057, fixes #2375 - OTRS migration currently do not support custom dropdown multi-select fields. --- .gitlab/ci/integration/otrs.yml | 18 ++++++++--------- app/models/object_manager/attribute.rb | 1 - lib/import/otrs/dynamic_field.rb | 5 +++++ lib/import/otrs/dynamic_field/dropdown.rb | 6 ++++++ lib/import/otrs/dynamic_field/multiselect.rb | 8 +++++++- lib/import/otrs/dynamic_field/text_area.rb | 7 ++++--- .../dropdown/without_possible_values.json | 20 +++++++++++++++++++ .../multiselect/without_possible_values.json | 20 +++++++++++++++++++ .../otrs/dynamic_field/text_area/default.json | 2 +- .../otrs/dynamic_field/dropdown_spec.rb | 10 ++++++++++ .../otrs/dynamic_field/multiselect_spec.rb | 12 ++++++++++- .../otrs/dynamic_field/text_area_spec.rb | 7 ++++--- test/integration/otrs_import_test.rb | 8 ++++---- 13 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 spec/fixtures/import/otrs/dynamic_field/dropdown/without_possible_values.json create mode 100644 spec/fixtures/import/otrs/dynamic_field/multiselect/without_possible_values.json diff --git a/.gitlab/ci/integration/otrs.yml b/.gitlab/ci/integration/otrs.yml index 5c0a94841..353289a4f 100644 --- a/.gitlab/ci/integration/otrs.yml +++ b/.gitlab/ci/integration/otrs.yml @@ -7,11 +7,11 @@ - echo -e "\\e[0Ksection_end:`date +%s`:zammad_db_unseeded\\r\\e[0K" - bundle exec rails test test/integration/otrs_import_test.rb variables: - FF_NETWORK_PER_BUILD: 1 # https://docs.gitlab.com/runner/configuration/feature-flags.html - IMPORT_OTRS_ENDPOINT: "http://zammad-ci-otrsimport-app/otrs/public.pl?Action=ZammadMigrator" - TZ: "Europe/Berlin" # Required for the zammad-ci-otrsimport-app containers + FF_NETWORK_PER_BUILD: 1 # https://docs.gitlab.com/runner/configuration/feature-flags.html + IMPORT_OTRS_ENDPOINT: 'http://zammad-ci-otrsimport-app/otrs/public.pl?Action=ZammadMigrator' + TZ: 'Europe/Berlin' # Required for the zammad-ci-otrsimport-app containers -"minitest:integration:otrs:6": +'minitest:integration:otrs:6': <<: *template_integration_otrs services: - name: $CI_REGISTRY/docker/zammad-mysql:stable @@ -23,7 +23,7 @@ - name: $CI_REGISTRY/docker/zammad-ci-otrsimport-app:otrs6 alias: zammad-ci-otrsimport-app -"minitest:integration:otrs:5": +'minitest:integration:otrs:5': <<: *template_integration_otrs services: - name: $CI_REGISTRY/docker/zammad-mysql:stable @@ -35,7 +35,7 @@ - name: $CI_REGISTRY/docker/zammad-ci-otrsimport-app:otrs5 alias: zammad-ci-otrsimport-app -"minitest:integration:otrs:4": +'minitest:integration:otrs:4': <<: *template_integration_otrs services: - name: $CI_REGISTRY/docker/zammad-mysql:stable @@ -47,7 +47,7 @@ - name: $CI_REGISTRY/docker/zammad-ci-otrsimport-app:otrs4 alias: zammad-ci-otrsimport-app -"minitest:integration:otrs:33": +'minitest:integration:otrs:33': <<: *template_integration_otrs services: - name: $CI_REGISTRY/docker/zammad-mysql:stable @@ -59,7 +59,7 @@ - name: $CI_REGISTRY/docker/zammad-ci-otrsimport-app:otrs33 alias: zammad-ci-otrsimport-app -"minitest:integration:otrs:32": +'minitest:integration:otrs:32': <<: *template_integration_otrs services: - name: $CI_REGISTRY/docker/zammad-mysql:stable @@ -71,7 +71,7 @@ - name: $CI_REGISTRY/docker/zammad-ci-otrsimport-app:otrs32 alias: zammad-ci-otrsimport-app -"minitest:integration:otrs:31": +'minitest:integration:otrs:31': <<: *template_integration_otrs services: - name: $CI_REGISTRY/docker/zammad-mysql:stable diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 6431497c6..ef711b29a 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -306,7 +306,6 @@ possible types =end def self.add(data) - force = data[:force] data.delete(:force) diff --git a/lib/import/otrs/dynamic_field.rb b/lib/import/otrs/dynamic_field.rb index 90fbf6558..d3183a1ef 100644 --- a/lib/import/otrs/dynamic_field.rb +++ b/lib/import/otrs/dynamic_field.rb @@ -8,6 +8,7 @@ module Import @internal_name = self.class.convert_name(dynamic_field['Name']) return if already_imported?(dynamic_field) + return if skip?(dynamic_field) initialize_attribute_config(dynamic_field) @@ -54,6 +55,10 @@ module Import } end + def skip?(_dynamic_field) + false + end + def add ObjectManager::Attribute.add(@attribute_config) ObjectManager::Attribute.migration_execute(false) diff --git a/lib/import/otrs/dynamic_field/dropdown.rb b/lib/import/otrs/dynamic_field/dropdown.rb index ab68d31c0..6dcd0a91e 100644 --- a/lib/import/otrs/dynamic_field/dropdown.rb +++ b/lib/import/otrs/dynamic_field/dropdown.rb @@ -17,6 +17,12 @@ module Import } ) end + + private + + def skip?(dynamic_field) + !dynamic_field['Config']['PossibleValues'] + end end end end diff --git a/lib/import/otrs/dynamic_field/multiselect.rb b/lib/import/otrs/dynamic_field/multiselect.rb index 4f80257e2..b3fd0a297 100644 --- a/lib/import/otrs/dynamic_field/multiselect.rb +++ b/lib/import/otrs/dynamic_field/multiselect.rb @@ -6,7 +6,7 @@ module Import class Multiselect < Import::OTRS::DynamicField def init_callback(dynamic_field) @attribute_config.merge!( - data_type: 'select', + data_type: 'multiselect', data_option: { default: '', multiple: true, @@ -17,6 +17,12 @@ module Import } ) end + + private + + def skip?(dynamic_field) + !dynamic_field['Config']['PossibleValues'] + end end end end diff --git a/lib/import/otrs/dynamic_field/text_area.rb b/lib/import/otrs/dynamic_field/text_area.rb index 17282fb6a..4929f4f85 100644 --- a/lib/import/otrs/dynamic_field/text_area.rb +++ b/lib/import/otrs/dynamic_field/text_area.rb @@ -8,9 +8,10 @@ module Import @attribute_config.merge!( data_type: 'textarea', data_option: { - default: dynamic_field['Config']['DefaultValue'], - rows: dynamic_field['Config']['Rows'], - null: true, + default: dynamic_field['Config']['DefaultValue'], + rows: dynamic_field['Config']['Rows'], + null: true, + maxlength: 3000, } ) end diff --git a/spec/fixtures/import/otrs/dynamic_field/dropdown/without_possible_values.json b/spec/fixtures/import/otrs/dynamic_field/dropdown/without_possible_values.json new file mode 100644 index 000000000..51f960980 --- /dev/null +++ b/spec/fixtures/import/otrs/dynamic_field/dropdown/without_possible_values.json @@ -0,0 +1,20 @@ +{ + "ID": "40", + "ChangeTime": "2016-05-25 11:14:06", + "InternalField": "0", + "ValidID": "1", + "CreateTime": "2014-08-21 14:54:15", + "Label": "Dropdown Example Without PossibleValues", + "FieldOrder": "30", + "Config": { + "TranslatableValues": "0", + "PossibleValues": null, + "TreeView": "0", + "DefaultValue": "", + "Link": "", + "PossibleNone": "1" + }, + "FieldType": "Dropdown", + "Name": "DropdownExampleWithoutPossibleValues", + "ObjectType": "Ticket" +} diff --git a/spec/fixtures/import/otrs/dynamic_field/multiselect/without_possible_values.json b/spec/fixtures/import/otrs/dynamic_field/multiselect/without_possible_values.json new file mode 100644 index 000000000..6b07fed60 --- /dev/null +++ b/spec/fixtures/import/otrs/dynamic_field/multiselect/without_possible_values.json @@ -0,0 +1,20 @@ +{ + "ID": "40", + "ChangeTime": "2016-05-25 11:14:06", + "InternalField": "0", + "ValidID": "1", + "CreateTime": "2014-08-21 14:54:15", + "Label": "Multiselect Example Without PossibleValues", + "FieldOrder": "30", + "Config": { + "TranslatableValues": "0", + "PossibleValues": null, + "TreeView": "0", + "DefaultValue": "", + "Link": "", + "PossibleNone": "1" + }, + "FieldType": "Multiselect", + "Name": "MultiselectExampleWithoutPossibleValues", + "ObjectType": "Ticket" +} diff --git a/spec/fixtures/import/otrs/dynamic_field/text_area/default.json b/spec/fixtures/import/otrs/dynamic_field/text_area/default.json index 1322f4448..3191170fa 100644 --- a/spec/fixtures/import/otrs/dynamic_field/text_area/default.json +++ b/spec/fixtures/import/otrs/dynamic_field/text_area/default.json @@ -14,4 +14,4 @@ "FieldType": "TextArea", "Name": "TextAreaExample", "ObjectType": "Ticket" -} \ No newline at end of file +} diff --git a/spec/lib/import/otrs/dynamic_field/dropdown_spec.rb b/spec/lib/import/otrs/dynamic_field/dropdown_spec.rb index 9b8315763..2ef222bf0 100644 --- a/spec/lib/import/otrs/dynamic_field/dropdown_spec.rb +++ b/spec/lib/import/otrs/dynamic_field/dropdown_spec.rb @@ -42,4 +42,14 @@ RSpec.describe Import::OTRS::DynamicField::Dropdown do dynamic_field_from_json('dropdown/default', zammad_structure) end + + context 'without possible values' do + it 'imports no field without possible value' do + allow(ObjectManager::Attribute).to receive(:add) + + described_class.new(load_dynamic_field_json('dropdown/without_possible_values')) + + expect(ObjectManager::Attribute).not_to have_received(:add) + end + end end diff --git a/spec/lib/import/otrs/dynamic_field/multiselect_spec.rb b/spec/lib/import/otrs/dynamic_field/multiselect_spec.rb index 71bf62ac1..c8e093ff3 100644 --- a/spec/lib/import/otrs/dynamic_field/multiselect_spec.rb +++ b/spec/lib/import/otrs/dynamic_field/multiselect_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Import::OTRS::DynamicField::Multiselect do position: '4', created_by_id: 1, updated_by_id: 1, - data_type: 'select', + data_type: 'multiselect', data_option: { default: '', multiple: true, @@ -42,4 +42,14 @@ RSpec.describe Import::OTRS::DynamicField::Multiselect do dynamic_field_from_json('multiselect/default', zammad_structure) end + + context 'without possible values' do + it 'imports no field without possible value' do + allow(ObjectManager::Attribute).to receive(:add) + + described_class.new(load_dynamic_field_json('multiselect/without_possible_values')) + + expect(ObjectManager::Attribute).not_to have_received(:add) + end + end end diff --git a/spec/lib/import/otrs/dynamic_field/text_area_spec.rb b/spec/lib/import/otrs/dynamic_field/text_area_spec.rb index 9eab5a054..ab89d637a 100644 --- a/spec/lib/import/otrs/dynamic_field/text_area_spec.rb +++ b/spec/lib/import/otrs/dynamic_field/text_area_spec.rb @@ -26,9 +26,10 @@ RSpec.describe Import::OTRS::DynamicField::TextArea do updated_by_id: 1, data_type: 'textarea', data_option: { - default: '', - rows: '20', - null: true + default: '', + rows: '20', + null: true, + maxlength: 3000, } } diff --git a/test/integration/otrs_import_test.rb b/test/integration/otrs_import_test.rb index 38cbb6d8a..1f6896fa0 100644 --- a/test/integration/otrs_import_test.rb +++ b/test/integration/otrs_import_test.rb @@ -36,7 +36,7 @@ class OtrsImportTest < ActiveSupport::TestCase end.collect do |local_object| local_object['name'] end - expected_object_attribute_names = %w[vertriebsweg te_test number sugar_crm_remote_no sugar_crm_company_selected_no sugar_crm_company_selection combine title itsm_criticality customer_id itsm_impact itsm_review_required itsm_decision_result organization_id itsm_repair_start_time itsm_recovery_start_time itsm_decision_date itsm_due_date topic_no open_exchange_ticket_number hostname ticket_free_key11 type ticket_free_text11 open_exchange_tn topic zarafa_tn group_id scom_hostname checkbox_example scom_uuid scom_state scom_service location owner_id department customer_location state_id pending_time priority_id tags] + expected_object_attribute_names = %w[vertriebsweg te_test number sugar_crm_remote_no sugar_crm_company_selected_no sugar_crm_company_selection combine title itsm_criticality customer_id itsm_impact itsm_review_required itsm_decision_result organization_id itsm_repair_start_time itsm_recovery_start_time itsm_decision_date itsm_due_date topic_no open_exchange_ticket_number hostname ticket_free_key11 type ticket_free_text11 open_exchange_tn topic zarafa_tn group_id scom_hostname checkbox_example scom_uuid scom_state scom_service location owner_id department customer_location textfeld state_id pending_time priority_id tags] assert_equal(expected_object_attribute_names, object_attribute_names, 'dynamic field names') end @@ -109,7 +109,7 @@ class OtrsImportTest < ActiveSupport::TestCase assert_equal('invalid-temp', user4.firstname) assert_equal('invalid-temp', user4.lastname) assert_equal('invalid-temp', user4.login) - assert_equal('invalid-temp@example.com', user4.email) + assert_equal(false, user4.active) assert(user4.roles.include?(role_agent)) @@ -217,10 +217,10 @@ class OtrsImportTest < ActiveSupport::TestCase ticket = Ticket.find(591) assert_equal('Some other smart subject!', ticket.title) assert_equal('488', ticket.vertriebsweg) - assert_equal('["193"]', ticket.te_test) # TODO: multiselect + assert_equal(%w[193 194], ticket.te_test) assert_equal('358', ticket.sugar_crm_remote_no) assert_equal('69', ticket.sugar_crm_company_selected_no) - assert_equal('["382"]', ticket.sugar_crm_company_selection) # TODO: multiselect + assert_equal(['382'], ticket.sugar_crm_company_selection) assert_equal('310', ticket.topic_no) assert_equal('495', ticket.open_exchange_ticket_number) assert_equal('208', ticket.hostname)