diff --git a/app/workers/import/jira_create_project_job/custom_field_names.rb b/app/workers/import/jira_create_project_job/custom_field_names.rb index 4bfd03a77fed..d7e129f26802 100644 --- a/app/workers/import/jira_create_project_job/custom_field_names.rb +++ b/app/workers/import/jira_create_project_job/custom_field_names.rb @@ -30,6 +30,9 @@ module Import class JiraCreateProjectJob module CustomFieldNames + # The counter `unique_name` appends when a name is taken and cannot be reused (e.g. int vs float type of the field) + NUMBERING_SUFFIX = /\s*\(\d+\)\z/ + module_function # Matches `Example`, `Example (2)`, `Example (DYX)` and `Example (DYX, ABC) @@ -41,6 +44,14 @@ def candidates(base_name) .order(:id) end + def same_field?(custom_field_name, import_name) + without_numbering(custom_field_name).casecmp?(import_name.to_s) + end + + def without_numbering(name) + name.to_s.sub(NUMBERING_SUFFIX, "") + end + def unique_name(name) unique_name = name suffix = 2 diff --git a/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb b/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb index f9146007fa9e..2416f224aad8 100644 --- a/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb +++ b/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb @@ -299,8 +299,19 @@ def compatible_multi_value?(custom_field) def best_matching_candidate(candidates, run_custom_field_ids) return best_value_bearing_candidate(candidates, run_custom_field_ids) if value_bearing_format? - candidates.find { |cf| cf.name.casecmp?(@import_name) } || - candidates.find { |cf| cf.name.casecmp?(stable_base_name) } + exact_name_match(candidates) || numbered_name_match(candidates) + end + + def exact_name_match(candidates) + import_names.filter_map { |name| candidates.find { |cf| cf.name.casecmp?(name) } }.first + end + + def numbered_name_match(candidates) + import_names.filter_map { |name| candidates.find { |cf| CustomFieldNames.same_field?(cf.name, name) } }.first + end + + def import_names + [@import_name, stable_base_name].uniq end def value_bearing_format? diff --git a/docs/installation-and-operations/jira-migration/custom-fields/README.md b/docs/installation-and-operations/jira-migration/custom-fields/README.md index 6ad5fa4883ab..75eaf11b9939 100644 --- a/docs/installation-and-operations/jira-migration/custom-fields/README.md +++ b/docs/installation-and-operations/jira-migration/custom-fields/README.md @@ -141,6 +141,9 @@ If an OpenProject custom field with the same name and format already exists (fro the migrator reuses it instead of creating a duplicate. The existing custom field is linked to the import and its values are preserved. If a name collision exists but the formats differ, the migrator appends a numeric suffix to the new field name (e.g., `My Field (2)`). +Such a numeric suffix is disregarded when the migrator looks for a field to reuse, so a later run reuses `My Field (2)` +instead of creating `My Field (3)`. Only a numeric suffix is treated this way - the project-key suffixes described under +[Field contexts](#field-contexts) stay part of the name. For **Hierarchy** and **List** fields, deduplication is attempted but not guaranteed because option lists may differ. A new custom field with a numeric suffix may be created for these types. diff --git a/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb b/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb index 84acb4e6c915..4990790d7f38 100644 --- a/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb +++ b/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb @@ -65,6 +65,38 @@ end end + describe ".same_field?" do + it "matches the same name regardless of case" do + expect(described_class).to be_same_field("Story Points", "STORY POINTS") + end + + it "matches a stored name carrying the counter unique_name appends" do + expect(described_class).to be_same_field("Story Points (2)", "Story Points") + expect(described_class).to be_same_field("Story Points (12)", "Story Points") + end + + it "strips the counter from the stored name only" do + expect(described_class).not_to be_same_field("Story Points", "Story Points (2)") + end + + it "matches a Jira field whose own name ends in a number against its renamed field" do + expect(described_class).to be_same_field("Story Points (2) (2)", "Story Points (2)") + end + + it "keeps a project key suffix apart from the counter" do + expect(described_class).not_to be_same_field("Story Points (XYZ)", "Story Points") + end + + it "matches a project key suffix with itself despite a counter" do + expect(described_class).to be_same_field("Story Points (XYZ) (2)", "Story Points (XYZ)") + end + + it "does not match a different name" do + expect(described_class).not_to be_same_field("Story Points Estimate", "Story Points") + expect(described_class).not_to be_same_field("Story Points 2", "Story Points") + end + end + describe ".unique_name" do subject(:unique_name) { described_class.unique_name("Resolution") } diff --git a/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb b/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb index 30fbfb1e8152..63fc61004d6d 100644 --- a/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb +++ b/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb @@ -981,21 +981,41 @@ def date_builder expect(date_builder.find_existing_custom_field).to eq(exact) end - it "does not reuse a suffixed candidate when no exact name matches" do - create(:date_wp_custom_field, name: "Due (archived)") + it "prefers the exact base name over a numbered candidate created before it" do create(:date_wp_custom_field, name: "Due (2)") + exact = create(:date_wp_custom_field, name: "Due") + + expect(date_builder.find_existing_custom_field).to eq(exact) + end + + it "reuses the numbered field an earlier run created when no exact name matches" do + create(:date_wp_custom_field, name: "Due (archived)") + numbered = create(:date_wp_custom_field, name: "Due (2)") + + expect(date_builder.find_existing_custom_field).to eq(numbered) + end + + it "ignores a candidate whose suffix is not the counter unique_name appends" do + create(:date_wp_custom_field, name: "Due (archived)") expect(date_builder.find_existing_custom_field).to be_nil end - it "keeps two Jira fields apart when one is named like the other's dedup suffix" do - suffixed = create(:string_wp_custom_field, name: "foo (2)") - schema = { "type" => "string", "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:textfield" } - builder = described_class.new(jira_field_for(name: "foo", schema:), jira_import:) + it "reuses the renamed field of a Jira field whose own name ends in a number" do + create(:string_wp_custom_field, name: "Due (2)") + renamed = create(:date_wp_custom_field, name: "Due (2) (2)") + schema = { "type" => "date", "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:datepicker" } + builder = described_class.new(jira_field_for(name: "Due (2)", schema:), jira_import:) + + expect(builder.find_existing_custom_field).to eq(renamed) + end + + it "does not let a Jira field whose name ends in a number take over the plain field" do + create(:date_wp_custom_field, name: "Due") + schema = { "type" => "date", "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:datepicker" } + builder = described_class.new(jira_field_for(name: "Due (2)", schema:), jira_import:) expect(builder.find_existing_custom_field).to be_nil - expect(builder.custom_field_settings.first).to eq("foo") - expect(suffixed.reload.name).to eq("foo (2)") end end diff --git a/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb b/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb index 95849b754afc..f02836220470 100644 --- a/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb +++ b/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb @@ -1384,4 +1384,61 @@ def per_issue_union(job, registry, jira_project) end end end + + describe "a custom field an earlier import run had to name with a counter" do + let!(:jira_field) do + create(:jira_field, jira_import:, + origin_id: "customfield_10254", + payload: { + "id" => "customfield_10254", + "name" => "CF Number", + "schema" => { + "type" => "number", + "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:float", + "customId" => 10254 + } + }) + end + let!(:jira_issue) do + create(:jira_issue, jira_import:, + origin_id: "10200", + jira_project:, + payload: issue_payload) + end + + # An existing field of an incompatible format cannot be reused, so the first run has to fall + # back to the next free name. + let!(:incompatible_custom_field) { create(:integer_wp_custom_field, name: "CF Number") } + + let(:second_jira_import) do + create(:jira_import, jira:, author:, + projects: [{ "id" => jira_project_id, "key" => jira_project_key, + "name" => jira_project_name }]) + end + + def run_second_import + second_jira_project = create(:jira_project, jira_import: second_jira_import, + origin_id: jira_project_id, + payload: jira_project_payload) + create(:jira_field, jira_import: second_jira_import, + origin_id: jira_field.origin_id, + payload: jira_field.payload) + create(:jira_issue, jira_import: second_jira_import, + origin_id: "10200", + jira_project: second_jira_project, + payload: issue_payload) + Import::JiraCreateCustomFieldsJob.perform_now(second_jira_import.id) + end + + before { import_project } + + it "names it with the next free counter" do + expect(WorkPackageCustomField.pluck(:name)).to contain_exactly("CF Number", "CF Number (2)") + end + + it "reuses it on a later import run instead of allocating the next counter" do + expect { run_second_import }.not_to change(WorkPackageCustomField, :count) + expect(WorkPackageCustomField.pluck(:name)).to contain_exactly("CF Number", "CF Number (2)") + end + end end